Skip to content

chore: Surfaces API error when autogen wait reaches a failed state - #4668

Merged
EspenAlbert merged 9 commits into
masterfrom
CLOUDP-435981_autogen-wait-error-description
Aug 26, 2026
Merged

chore: Surfaces API error when autogen wait reaches a failed state#4668
EspenAlbert merged 9 commits into
masterfrom
CLOUDP-435981_autogen-wait-error-description

Conversation

@EspenAlbert

@EspenAlbert EspenAlbert commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds autogen error_description_property so wait failures can surface the API error string (for example errorMessage) instead of only operation failed with state "FAILED". Wait errors also include an import-style resource id when id_attributes are configured, and create-timeout cleanup still runs when that timeout is wrapped.

Link to any related issue(s): CLOUDP-435981, CLOUDP-437425

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contributing guides
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fix and verified my code
  • If changes include deprecations or removals I have added appropriate changelog entries.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

Codegen emits ErrorDescriptionProperty and IDAttributes on generated wait blocks. Existing wait resources (search_index_api, stream_processor_api) get IDAttributes so failed waits name the resource. HandleCreateTimeout uses errors.As so a wrapped *retry.TimeoutError still triggers cleanup.

Example Errors from #4671

Example timeout message

  | Error: Error waiting for changes in Create
  |
  |   with mongodbatlas_cloud_backup_collection_restore_job.test,
  |   on terraform_plugin_test.tf line 44, in resource "mongodbatlas_cloud_backup_collection_restore_job" "test":
  |   44: \tresource "mongodbatlas_cloud_backup_collection_restore_job" "test" {
  |
  | project_id="6a8e9dee4d559341692a538f",
  | cluster_name="test-acc-tf-c-933326487156066343",
  | job_id="6a8ea57c4d559341692c2e9f": timeout while waiting for state to become
  | 'SUCCESSFUL' (last state: 'INITIALIZING', timeout: 1m0s)

Example failed message.

  | Error: Error waiting for changes in Create
  |
  |   with mongodbatlas_cloud_backup_collection_restore_job.test,
  |   on terraform_plugin_test.tf line 44, in resource "mongodbatlas_cloud_backup_collection_restore_job" "test":
  |   44: \tresource "mongodbatlas_cloud_backup_collection_restore_job" "test" {
  |
  | project_id="6a8e9dee4d559341692a538f",
  | cluster_name="test-acc-tf-c-933326487156066343",
  | job_id="6a8ea553e381fc41afe39d36": Operation failed with state "FAILED", wanted target "SUCCESSFUL". The restore could not complete because 1
  | collection was not found. All changes have been rolled back. Please retry the
  | restore or contact Support if the issue continues.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds richer autogenerated wait failures with API error details and resource IDs, while preserving cleanup for wrapped timeout errors.

Changes:

  • Propagates error-description and ID configuration through code generation.
  • Formats failed waits with API messages and import-style resource IDs.
  • Adds timeout-cleanup, formatter, and generation tests.

Reviewed changes

Copilot reviewed 12 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary
tools/codegen/gofilegen/resource/testdata/wait-error-description.golden.go Golden output for error descriptions.
tools/codegen/gofilegen/resource/testdata/wait-configuration.golden.go Golden output for wait IDs.
tools/codegen/gofilegen/resource/resource_file.go Maps wait configuration into generated resources.
tools/codegen/gofilegen/resource/resource_file_test.go Tests generated wait configuration.
tools/codegen/gofilegen/codetemplate/template.go Extends the wait template model.
tools/codegen/gofilegen/codetemplate/resource-file.go.tmpl Emits wait error and ID fields. Review: regenerate all affected wait resources or make ID emission conditional to avoid generation drift.
tools/codegen/config/config_model.go Adds YAML error-property configuration. Review: no production wait configuration currently sets error_description_property, so the described behavior is unreachable; configure affected waits and regenerate, or narrow the PR description.
tools/codegen/codespec/model.go Adds the code-spec error-property field.
tools/codegen/codespec/api_to_provider_spec_mapper.go Maps wait configuration to the code specification.
internal/serviceapi/streamprocessorapi/resource.go Adds resource identifiers to stream processor waits.
internal/serviceapi/searchindexapi/resource.go Adds resource identifiers to search index waits.
internal/common/cleanup/handle_timeout.go Detects wrapped timeout errors for cleanup.
internal/common/cleanup/handle_timeout_test.go Tests wrapped timeout cleanup.
internal/common/autogen/wait_failure.go Formats wait failures and resource IDs.
internal/common/autogen/wait_failure_test.go Tests failure formatting and IDs.
internal/common/autogen/handle_operations.go Captures and formats polling failures.
Files not reviewed (4)
  • internal/serviceapi/searchindexapi/resource.go: Generated file
  • internal/serviceapi/streamprocessorapi/resource.go: Generated file
  • tools/codegen/gofilegen/resource/testdata/wait-configuration.golden.go: Generated file
  • tools/codegen/gofilegen/resource/testdata/wait-error-description.golden.go: Generated file
Suppressed comments (2)

internal/common/cleanup/handle_timeout.go:25

  • This PR contains a bug fix and an enhancement, but it adds no .changelog/<PR_NUMBER>.txt release-note entry. The repository's release-note convention requires changelog entries for both bug fixes and new features, so please add the appropriate entry before merging.
	if !errors.As(errWait, &timeoutErr) {

tools/codegen/codespec/api_to_provider_spec_mapper.go:262

  • The new golden test builds a codespec.Wait directly, so it does not exercise the new config.Wait to codespec.Wait mapping. A YAML/tag or mapper regression could silently drop error_description_property while the golden test still passes. Please add a mapper fixture/assertion covering this field.
		ErrorDescriptionProperty: waitConfig.ErrorDescriptionProperty,

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/codegen/config/config_model.go
Comment thread tools/codegen/gofilegen/codetemplate/resource-file.go.tmpl Outdated
@EspenAlbert EspenAlbert changed the title feat: Surfaces API error when autogen wait reaches a failed state chore: Surfaces API error when autogen wait reaches a failed state Aug 24, 2026
@EspenAlbert
EspenAlbert marked this pull request as ready for review August 24, 2026 09:49
@EspenAlbert
EspenAlbert requested a review from a team as a code owner August 24, 2026 09:49
@EspenAlbert
EspenAlbert requested a lite review from Copilot August 24, 2026 09:50
@augmentcode

augmentcode Bot commented Aug 24, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR improves diagnostics from autogenerated long-running-operation waits.

Changes:

  • Extends `WaitReq` with an optional API error-description property and resource ID attributes.
  • Captures the final poll response/state and formats failures with a configured API error message.
  • Prefixes wait failures with named import-style resource identifiers when available.
  • Preserves `*retry.TimeoutError` wrapping so create-timeout cleanup still recognizes timeouts.
  • Updates create-timeout handling to use `errors.As` for wrapped timeout errors.
  • Regenerates wait-enabled service API resources to emit their ID attributes.
  • Threads `error_description_property` through config, code-spec mapping, and Go templates.
  • Adds unit and golden-generation coverage for failure formatting and generated wait configuration.
Technical Notes: Failed terminal states can now surface an API field such as `errorMessage`; timeouts retain their original error chain while gaining identifier context.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 25 changed files in this pull request and generated 1 comment.

Files not reviewed (11)
  • internal/serviceapi/clusterapi/resource.go: Generated file
  • internal/serviceapi/clusteroldapi/resource.go: Generated file
  • internal/serviceapi/privatelinkendpointservicedatafederationonlinearchive/resource.go: Generated file
  • internal/serviceapi/pushbasedlogexportapi/resource.go: Generated file
  • internal/serviceapi/searchdeploymentapi/resource.go: Generated file
  • internal/serviceapi/searchindexapi/resource.go: Generated file
  • internal/serviceapi/streamconnectionapi/resource.go: Generated file
  • internal/serviceapi/streamconnectionfailover/resource.go: Generated file
  • internal/serviceapi/streamprocessorapi/resource.go: Generated file
  • tools/codegen/gofilegen/resource/testdata/wait-configuration.golden.go: Generated file
  • tools/codegen/gofilegen/resource/testdata/wait-error-description.golden.go: Generated file
Suppressed comments (3)

internal/common/cleanup/handle_timeout.go:25

  • This PR introduces operator-visible provider bug-fix/enhancement behavior (wait diagnostics now expose API failures and create-timeout cleanup handles wrapped errors), but no .changelog/<PR_NUMBER>.txt entry is included. The repository changelog process requires release-note:bug or release-note:enhancement entries for these changes; please add the provider-level entry or explicitly document why this internal plumbing is excluded.
	if !errors.As(errWait, &timeoutErr) {

tools/codegen/config/config_model.go:38

  • This bug fix/new wait behavior has no .changelog/<PR_NUMBER>.txt entry in the PR. The provider's release-note requirements call for a changelog entry for bug fixes and new features; please add one describing the surfaced API wait errors before merging.
	ErrorDescriptionProperty string   `yaml:"error_description_property"` // defined in camel case as found in API response body, e.g. "errorMessage"

tools/codegen/gofilegen/codetemplate/resource-file.go.tmpl:160

  • The generated update handler passes req.Plan to the wait path, while this list can contain computed-only path attributes. For example, streamconnectionfailover includes failover_connection_id, which is Computed and is read from state for the update request; it is not guaranteed to be known in the plan. On an update failure this produces an incomplete prefix such as failover_connection_id="" (and the same plan-backed model is used by readAPICallParams for polling). Use a state-backed model for the wait's identity/path parameters, or merge the state IDs before polling and formatting the failure.
			IDAttributes: []string{ {{range $.IDAttributes }}"{{ . }}", {{- end }} },

Comment thread internal/common/autogen/wait_failure.go
@EspenAlbert

Copy link
Copy Markdown
Collaborator Author

TestAccSearchIndexAPI_basic failed in PopulateWithSampleData (AccTest helper) while waiting for the Atlas sample-dataset job: unexpected state 'FAILED', wanted target 'COMPLETED'. The other 8 search_index_api tests then hit ATLAS_SEARCH_COLLECTION_NOT_FOUND for listingsAndReviews on the same shared cluster. That is HTTP 400 on Create, before any wait.

This PR only changes autogen wait error text (error_description_property, IDAttributes) and HandleCreateTimeout wrapping. None of that runs for the sample-dataset job. Resources that do use the new wait path passed (clusterapi, searchdeploymentapi, streamprocessorapi), including TestAccClusterAPI_deleteOnCreateTimeout.

Comment thread internal/common/autogen/handle_operations.go
Comment thread internal/common/autogen/wait_failure.go Outdated
Comment on lines +24 to +25
var timeoutErr *retry.TimeoutError
if !errors.As(errWait, &timeoutErr) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

refactoring or actual behaviour difference?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Behavior change. DefaultFormatWaitFailure wraps *retry.TimeoutError with %w so the id prefix is on the message. A type assert would miss the wrap and skip delete_on_create_timeout cleanup.

Comment thread internal/common/autogen/wait_failure.go Outdated
Comment thread internal/common/autogen/handle_operations.go
@EspenAlbert
EspenAlbert force-pushed the CLOUDP-435981_autogen-wait-error-description branch from 49c8a9b to 1467c8f Compare August 25, 2026 16:33

@AgustinBettati AgustinBettati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To consider: How much complexity is adding id attributes in the error message adding? Maybe we can leave this as a follow up, either way I believe terraform error are clear on which resource they are associated to.

Comment thread dev.tfrc Outdated
Comment thread tools/codegen/gofilegen/resource/resource_file.go
Comment thread internal/common/autogen/handle_operations.go
Comment thread internal/common/autogen/handle_operations.go
Comment thread internal/common/autogen/wait_failure.go Outdated

@AgustinBettati AgustinBettati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the follow ups

@EspenAlbert
EspenAlbert force-pushed the CLOUDP-435981_autogen-wait-error-description branch from a5dd3dc to c9f193d Compare August 26, 2026 13:39
@EspenAlbert
EspenAlbert merged commit d605994 into master Aug 26, 2026
51 of 52 checks passed
@EspenAlbert
EspenAlbert deleted the CLOUDP-435981_autogen-wait-error-description branch August 26, 2026 14:48
AgustinBettati added a commit that referenced this pull request Aug 26, 2026
Resolve handle_operations.go conflict: keep both master's waitRefreshResult
(#4668) and the ErrNotFound-based notFound. Declare IDLE as a pending state
in the refreshFunc test, now required by waitRefreshResult.
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