Skip to content

Conversation

@SarahFrench
Copy link
Member

@SarahFrench SarahFrench commented Oct 10, 2025

Following this feedback: #37732 (comment)

This PR replaces const strings, used to construct errors when using backends (incl. cloud block), with custom errors and custom pre-made diagnostics.

Custom errors are used where:

  • The error is actually returned from a method to some calling code (errBackendNoExistingWorkspaces)
    • An error, rather than a diag is needed for comparison.
  • The original code appended an error into a Diagnostics collection, so the new code will do the same.
    • If we replaced appending of an error with appending of a diagnostic then the rendered output in the terminal will be different, and would need a Summary value.

Pre-made diagnostics are used where:

  • The error is only created to then be interpolated into a diagnostic's Detail value
  • The error is consistently used with a diagnostic with the same Severity and Summary text.

Target Release

N/A

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@SarahFrench SarahFrench added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Oct 10, 2025
@SarahFrench SarahFrench changed the title Refactor backend errs as custom errors Refactor backend error strings as custom errors or pre-made diagnostics Oct 10, 2025
@SarahFrench
Copy link
Member Author

SarahFrench commented Oct 10, 2025

Something to bear in mind is whether these changes could impact any parsing of human-readable output by HCP Terraform.

I believe these changes don't impact output for a few reasons:

  • Continuing to append errors to diagnostics collections where that was done before (versus appending an error diagnostic)
  • Use of strings.TrimSpace made unnecessary by removing the extra newlines at the start and end of the messages
  • These changes don't impact how the final message to stdout happens, and also shouldn't impact the content of those messages.

However if there are any further tests that would be useful to ensure that's true, I'd need to guidance about where to start, if needed.

@SarahFrench SarahFrench marked this pull request as ready for review October 10, 2025 21:18
@SarahFrench SarahFrench requested a review from a team as a code owner October 10, 2025 21:18
Comment on lines -1546 to +1525
diags = diags.Append(err)
diags = diags.Append(errBackendWriteSavedDiag(err))
}
// No need to call PersistState as it's a no-op
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting this - this is a place where I have actually changed something about the output from Terraform. This error wasn't 'wrapped' in the same way that other errors (from WriteState or PersistState) were handled elsewhere in the code.

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the refactoring 👍🏻

@SarahFrench SarahFrench merged commit d11bdf6 into main Oct 13, 2025
18 of 19 checks passed
@SarahFrench SarahFrench deleted the refactor-backend-errs-as-custom-errors branch October 13, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog-needed Add this to your PR if the change does not require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants