feat: surface 429s as typed BugSplatRateLimitError#168
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves rate-limit handling by introducing a typed BugSplatRateLimitError (including parsed Retry-After seconds) and by plumbing HTTP response headers through BugSplatResponse so callers can make informed backoff decisions across both cookie-auth and OAuth client paths.
Changes:
- Added
BugSplatRateLimitErrorplusparseRetryAfterSeconds()to parseRetry-Afterand exposeretryAfterSecondsto consumers. - Extended
BugSplatResponseto optionally includeheaders, and updated both API client implementations to pass throughresponse.headers. - Updated symbols/versions/crash-post clients to throw the typed error on HTTP 429, with new/expanded specs (symbols + versions + common helper).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/versions/versions-api-client/versions-api-client.ts | Throw BugSplatRateLimitError on 429 and parse Retry-After. |
| src/versions/versions-api-client/versions-api-client.spec.ts | Add 429 test asserting typed error + retryAfterSeconds. |
| src/symbols/symbols-api-client/symbols-api-client.ts | Throw BugSplatRateLimitError on 429 and parse Retry-After. |
| src/symbols/symbols-api-client/symbols-api-client.spec.ts | Add 429 test asserting typed error + retryAfterSeconds. |
| src/post/crash-post-client.ts | Throw BugSplatRateLimitError on 429 and parse Retry-After. |
| src/common/client/oauth-client-credentials-api-client/oauth-client-credentials-api-client.ts | Pass response.headers through in returned BugSplatResponse. |
| src/common/client/index.ts | Re-export new error/type/helper from the client barrel. |
| src/common/client/bugsplat-api-client/bugsplat-api-client.ts | Pass response.headers through in returned BugSplatResponse. |
| src/common/client/api-client.ts | Add headers to BugSplatResponse, add BugSplatRateLimitError, add parseRetryAfterSeconds. |
| src/common/client/api-client.spec.ts | Add unit tests for the new error type and parseRetryAfterSeconds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rate-limited (HTTP 429) responses were thrown as a generic Error with a "too many requests" message, leaving consumers to match on the message string to detect them. Add a BugSplatRateLimitError (isRateLimitError marker + status) and throw it from the three presigned-URL sites (symbols/versions getPresignedUrl, crash-post getCrashUploadUrl) so consumers can detect rate limiting by type. Exported via the common/client barrel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bobbyg603
changed the title
bobbyg603
force-pushed
the
feat/rate-limit-error
branch
from
10398f7 to
089d685
Compare
daveplunkett
approved these changes
Jun 8, 2026
2 tasks
Addresses Copilot review feedback on #168: - Remove trailing comma after the BugSplatRateLimitError `status` constructor param (incompatible with Prettier `trailingComma: es5`). - Add a spec asserting CrashPostClient throws BugSplatRateLimitError with status 429 when getCrashUploadUrl returns 429. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Husky's hook bootstrap is deprecated (fails in v10), and its commit-msg hook only linted individual branch commits that get squashed away on merge. Since PRs squash-merge with the PR title as the commit subject, that title is what semantic-release parses for version bumps. - Remove husky (dep, `prepare` script, .husky/, core.hooksPath). - Remove @commitlint/cli, @commitlint/config-conventional, .commitlintrc.js (only ever invoked by the husky commit-msg hook). - Add a PR Title workflow using amannn/action-semantic-pull-request to validate the PR title against Conventional Commits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bobbyg603
pushed a commit
that referenced
this pull request
Jun 8, 2026
# [15.2.0](v15.1.0...v15.2.0) (2026-06-08) ### Features * surface 429s as typed BugSplatRateLimitError ([#168](#168)) ([d7afcb1](d7afcb1))
Member
Author
|
🎉 This PR is included in version 15.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Rate-limited (HTTP 429) responses are currently thrown as a generic
Error('...too many requests'), leaving consumers (e.g. symbol-upload #184) to match on the message string to detect them — brittle and easy to break.What
BugSplatRateLimitError(src/common/client/api-client.ts) with anisRateLimitErrormarker andstatus, mirroring the existingBugSplatAuthenticationErrorpattern. Exported via thecommon/clientbarrel.SymbolsApiClient.getPresignedUrl,VersionsApiClient.getPresignedUrl, andCrashPostClient.getCrashUploadUrl.Non-breaking: new export + a more specific error subclass; the error messages and throw-based control flow are unchanged.
Not included
No
Retry-Afterhandling. AWS WAF doesn't emitRetry-Afterby default and its custom response headers are static (can't reflect the real remaining window), so consumers can't rely on it — symbol-upload backs off with a shared exponential gate instead. If we ever want an accurate hint, the right place to emit it is the application layer, and we can add it then.Tests
api-client.spec.tscovering the error shape (isRateLimitError, defaultstatus429).🤖 Generated with Claude Code