ShouldRetry callbacks have unnamed error parameter, preventing retries

[tjouni]

Summary

Several ShouldRetry callbacks in retry.Config have an unnamed function parameter:

ShouldRetry: func(error) bool {
    if awserrors.Matches(err, "InvalidAMIID.NotFound", "") {

Because the error parameter is unnamed, the callback references the outer scope's err variable (from a previous assignment) instead of the error passed by the retry framework. This means awserrors.Matches() evaluates the wrong error, and ShouldRetry always returns false for transient errors, causing the retry loop to exit immediately.

The correct pattern (used elsewhere in the same codebase) names the parameter:

ShouldRetry: func(err error) bool {
    if awserrors.Matches(err, "InvalidAMIID.NotFound", "") {

Affected Files

builder/common/ (AWS SDK v1)

• step_create_tags.go line 105 — checks InvalidAMIID.NotFound and InvalidSnapshot.NotFound
• step_run_source_instance.go line 367 — checks InvalidInstanceID.NotFound
• step_stop_ebs_instance.go line 48 — checks InvalidInstanceID.NotFound
• step_run_spot_instance.go line 513 — cosmetic only (return false)

common/ (AWS SDK v2)

• step_create_tags.go line 109 — same as above
• step_run_source_instance.go line 371 — same as above
• step_stop_ebs_instance.go line 51 — same as above
• step_run_spot_instance.go line 518 — cosmetic only (return false)

Impact

Users experience transient AWS API failures (e.g., InvalidAMIID.NotFound due to eventual consistency) that should be retried but are not. The build fails immediately instead of retrying up to 11 times as intended.

Evidence

This is likely the root cause of hashicorp/packer#10370 ("Intermittent errors tagging AWS spot instances"), where a user reported that CloudTrail showed only 1 API call despite the retry config specifying 11 tries.

Correct Pattern (reference)

The following files in the same codebase correctly name the parameter:

• builder/common/helper_funcs.go: func(err error) bool { ... }
• builder/common/step_pre_validate.go: func(err error) bool { ... }
• builder/common/step_run_source_instance.go lines 287, 325: func(err error) bool { ... }
• builder/ebs/step_create_ami.go: func(err error) bool { ... }