Skip to content

feat: show rate-limit UI feedback during 429 retry backoff#612

Open
danskmt wants to merge 1 commit into
mainfrom
feat/CLI-1512-rate-limit-ui-feedback-v2
Open

feat: show rate-limit UI feedback during 429 retry backoff#612
danskmt wants to merge 1 commit into
mainfrom
feat/CLI-1512-rate-limit-ui-feedback-v2

Conversation

@danskmt
Copy link
Copy Markdown
Contributor

@danskmt danskmt commented May 22, 2026

Description

When the CLI is rate-limited (HTTP 429), retry backoff previously happened silently, making the CLI look hung. This PR adds user-visible feedback during 429 retry waits and records retry rate-limit events in v2 instrumentation.

During 429 backoff (between attempts):

  • RetryMiddleware wraps retry failures in RetryAttemptError and invokes an optional RetryNotifyFunc via backoff.WithNotify.
  • networkImpl.handleRetryNotify maps 429 attempts to a catalog TooManyRequestsError (Level: warn, detail: Waiting up to Xs before retry (attempt N/M).) and shows it via OutputError.
  • Only 429 is surfaced; other retried status codes (5xx, etc.) do not produce this warn.

Telemetry:

  • Per workflow invocation, engineimpl.go wires SetRetryNotify on the cloned network access to call AddError with the catalog 429 warn, which appears in v2 interaction.errors as SNYK-0001.

When 429 retries are exhausted:

  • RetryMiddleware still returns the last 429 response with nil error (unchanged).
  • Downstream ResponseMiddleware maps 429 to SNYK-0001 (TooManyRequestsError, level error) — covered by an added test in response_test.go; response.go is unchanged.

Plumbing:

  • retry_middleware.go: RetryAttemptError, RetryNotifyFunc, optional callback on NewRetryMiddleware, notifyRetry
  • networking.go: RetryFeedbackNetworkAccess optional interface, handleRetryNotify, CatalogNotificationFromRetryAttempt, UI + callback wiring on networkImpl
  • engineimpl.go: SetUserInterface on network access; per-invocation SetRetryNotify for instrumentation
  • pkg/mocks/networking.go: MockRetryFeedbackNetworkAccess
  • Tests: TestCatalogNotificationFromRetryAttempt, retry callback/exhaustion tests, 429 mapping in response_test.go

Checklist

  • Tests added and all succeed (make test)
  • Regenerated mocks, etc. (make generate)
  • Linted (make lint)
  • Test your changes work for the CLI
    1. Clone / pull the latest CLI main.
    2. Run go get github.com/snyk/go-application-framework@b2a5217339f862aaabf928c17c1429a0cec9b549 in the cliv2 directory.
      • Tip: for local testing, you can uncomment the line near the bottom of the CLI's go.mod to point to your local GAF code.
    3. Run go mod tidy in the cliv2 directory.
    4. Run the CLI tests and do any required manual testing.
    5. Open a PR in the CLI repo now with the go.mod and go.sum changes.
    • Once this PR is merged, repeat these steps pointing at the latest GAF commit on main and update your CLI PR.

CLI PR: snyk/cli#6836

@danskmt danskmt requested review from a team as code owners May 22, 2026 09:44
@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 22, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 22, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from d01c9f2 to 3e467d9 Compare May 22, 2026 09:50
@snyk-pr-review-bot

This comment has been minimized.

Comment thread pkg/networking/middleware/retry_middleware_test.go Outdated
@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 3e467d9 to 72ba2b7 Compare May 22, 2026 10:21
@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 72ba2b7 to db8094f Compare May 22, 2026 11:31
@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from db8094f to 0619f04 Compare May 22, 2026 11:54
@danskmt danskmt mentioned this pull request May 22, 2026
9 tasks
@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 0619f04 to 158a66d Compare May 25, 2026 08:56
@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 158a66d to 7deee91 Compare May 25, 2026 11:20
@snyk-pr-review-bot

This comment has been minimized.

Comment thread pkg/networking/middleware/retry_middleware.go
Comment thread pkg/networking/middleware/retry_middleware.go Outdated
Comment thread pkg/networking/middleware/retry_middleware.go Outdated
Comment thread pkg/networking/middleware/retry_middleware_test.go Outdated
@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 7deee91 to 948059b Compare May 25, 2026 13:18
@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from 948059b to e6185df Compare May 25, 2026 13:52
@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the feat/CLI-1512-rate-limit-ui-feedback-v2 branch from e6185df to b2a5217 Compare May 26, 2026 07:49
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Breaking Change 🟡 [minor]

The signature of NewRetryMiddleware has been changed to return http.RoundTripper instead of *RetryMiddleware. While this is architecturally cleaner, it is a breaking change for any external packages that might have been casting the result or relying on the concrete pointer type. Internal callers have been updated to match the new variadic signature.

func NewRetryMiddleware(config configuration.Configuration, logger *zerolog.Logger, roundTripper http.RoundTripper, onRetryNotify ...RetryNotifyFunc) http.RoundTripper {
📚 Repository Context Analyzed

This review considered 22 relevant code sections from 15 files (average relevance: 0.99)

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.

2 participants