Atlantis API controller loses nested error messages from ProjectResults when terraform plans/applys fail

[johnvandeweghe]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Overview of the Issue

The APIs for both /plan and /apply return an error status code but do not properly json.Unmarshal the ProjectResult.error into a string. This makes determining what went wrong through the API impossible.

Reproduction Steps

1. Set up an API key.
2. Send a request to plan a project that fails because of a terraform issue.
3. Get a 500 but no useful error/TF output:
   Example response:

  {"Error":null,"Failure":"","ProjectResults":[{"Command":0,"SubCommand":"","RepoRelDir":"module-name-here","Workspace":"some-workspace","Error":{},"Failure":"","PlanSuccess":null,"PolicyCheckResults":null,"ApplySuccess":"","VersionSuccess":"","ImportSuccess":null,"StateRmSuccess":null,"ProjectName":"pr-12292","SilencePRComments":null}],"PlansDeleted":false}

Note the ProjectResults[].Error being a marshalled empty struct instead of the error string.

Environment details

This applies to the latest version of atlantis (and likely all previous versions since the API endpoints were added) with the default configuration.

Additional Context

This comes from this snippet (this one is from /plan, but /apply is very similar and has the same problem):

	result, err := a.apiPlan(request, ctx)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck
	if result.HasErrors() {
		code = http.StatusInternalServerError
	}


	// TODO: make a better response
	response, err := json.Marshal(result)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	a.respond(w, logging.Warn, code, "%s", string(response))

Not that err from the a.apiPlan will only populate from hooks, the actual plan errors are captured in the result, and detected by the HasErrors() call.

The most friendly fix would probably be something like a custom Unmarshal on the ProjectResults that converts the error to a string

I do not frequently write Golang and am just an Atlantis user, but I could attempt a fix if there is nobody more experienced available to jump on this.

[dosubot]

@johnvandeweghe Hello there! I'm here to help you with any bugs, questions, or contributions. Let's tackle this together!

To address the issue of nested error messages not being properly unmarshalled and displayed in the Atlantis API controller, you can modify the error handling logic to ensure that errors within ProjectResults are correctly extracted and formatted. Here's a suggested approach:

1. Update the apiReportError Function: Modify this function to handle nested error messages from ProjectResults. This involves checking if the error is of type *command.Result and then extracting the error messages from each ProjectResult.

2. Modify the Plan and Apply Methods: Ensure these methods use the updated error reporting logic to handle errors from the ProjectResults.

Here's an example implementation:

func (a *APIController) apiReportError(w http.ResponseWriter, code int, err error) {
	var errorMsg string
	if nestedErr, ok := err.(*command.Result); ok && nestedErr.HasErrors() {
		// Extract nested error messages from ProjectResults
		var errorMessages []string
		for _, projectResult := range nestedErr.ProjectResults {
			if projectResult.Error != nil {
				errorMessages = append(errorMessages, projectResult.Error.Error())
			}
		}
		errorMsg = strings.Join(errorMessages, "; ")
	} else {
		errorMsg = err.Error()
	}

	response, _ := json.Marshal(map[string]string{
		"error": errorMsg,
	})
	a.respond(w, logging.Warn, code, "%s", string(response))
}

func (a *APIController) Plan(w http.ResponseWriter, r *http.Request) {
	w.Header().Set("Content-Type", "application/json")

	request, ctx, code, err := a.apiParseAndValidate(r)
	if err != nil {
		a.apiReportError(w, code, err)
		return
	}

	result, err := a.apiPlan(request, ctx)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck
	if result.HasErrors() {
		a.apiReportError(w, http.StatusInternalServerError, result)
		return
	}

	response, err := json.Marshal(result)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	a.respond(w, logging.Warn, code, "%s", string(response))
}

func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) {
	w.Header().Set("Content-Type", "application/json")

	request, ctx, code, err := a.apiParseAndValidate(r)
	if err != nil {
		a.apiReportError(w, code, err)
		return
	}

	_, err = a.apiPlan(request, ctx)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck

	result, err := a.apiApply(request, ctx)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	if result.HasErrors() {
		a.apiReportError(w, http.StatusInternalServerError, result)
		return
	}

	response, err := json.Marshal(result)
	if err != nil {
		a.apiReportError(w, http.StatusInternalServerError, err)
		return
	}
	a.respond(w, logging.Warn, code, "%s", string(response))
}

This code ensures that nested errors are properly extracted and included in the API response, making it easier to diagnose issues when Terraform plans or applies fail ^[1].

To continue talking to Dosu, mention @dosu.

---

^{Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other}