Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update an error msg to avoid potential confusion #864

Merged
merged 2 commits into from
Mar 8, 2024
Merged

Conversation

uturunku1
Copy link
Collaborator

@uturunku1 uturunku1 commented Mar 4, 2024

Description

This go-tfe error workspace is still being processed to discover resources in combination with this provider-tfe error To delete this workspace without destroying the managed resources, add force_delete = true to the resource config created confusion in a customer.

He thought that the latter error msg was saying that his workspace had resources, when in reality he had no resources. And then he thought that the former error msg was confirming that he had resources associated with that workspace, and that they were being processed.

The customer had the intuition that perhaps these were errors related to having a state version that is in middle of processing changes, but because the errors were saying something different, he opened a ticket with Support.

The change that I am making here is to clarify that, yes, what is "still being processed" is the state version. I was able to verify this by looking at the context where the atlas error is created.

And then I am making this change to provider-tfe to mention that force-delete option can be used for when a state version is being process or when a workspace has resources: hashicorp/terraform-provider-tfe#1274

Testing plan

External links

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" go test ./... -v -run TestFunctionsAffectedByChange

...

@uturunku1 uturunku1 changed the title update error message regarding a state version still being processed.… Update an error msg to avoid potential confusion Mar 4, 2024
@uturunku1 uturunku1 marked this pull request as ready for review March 4, 2024 17:18
@uturunku1 uturunku1 requested a review from a team as a code owner March 4, 2024 17:18
errors.go Outdated Show resolved Hide resolved
@uturunku1 uturunku1 requested a review from brandonc March 6, 2024 00:15
brandonc
brandonc previously approved these changes Mar 7, 2024
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

Can you please mention in the CHANGELOG that we changed the message within ErrWorkspaceStillProcessing? This might be a string that folks depend on.

tfe.go Outdated
@@ -942,7 +942,7 @@ func checkResponseCode(r *http.Response) error {
return ErrWorkspaceLockedCannotDelete
}
if errorPayloadContains(errs, "being processed") {
return ErrWorkspaceStillProcessing
return ErrWorkspaceStillProcessing // as of 2024-03-04, the error message coming from atlas is "Latest workspace state is being processed to discover resources, please try again later"
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment would be useful to include in the godocs; you can move this comment above the definition for ErrWorkspaceStillProcessing. I would also change atlas to API since that might lead to some confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I agree that I should say "the API" instead of Atlas. Thanks!
I am going to remove this comment though. It was meant for internal usage (for those who contribute to Atlas.) But as you mention, Atlas is irrelevant to the Public. So, for that reason, that comment also shouldn't be added to the godocs.

… Before the msg did not include the word state, so a customer, who had no resources in the workspace, misinterpret the msg and thought that it was saying that he had resources in the workspace and they were being processed
@uturunku1 uturunku1 merged commit 418463a into main Mar 8, 2024
10 checks passed
@uturunku1 uturunku1 deleted the update-error-msg branch March 8, 2024 22:16
Copy link

github-actions bot commented Mar 8, 2024

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants