create: report GitHub 5xx as a GitHub error; skip live-GitHub tests on it - #34419
Conversation
|
Updated 10:05 PM PT - Jul 18th, 2026
@autofix-ci[bot], your commit 818ceee is building: |
|
Reproduced locally during the outage: Second commit routes 429 into the existing rate-limit arm (so it gets the The one-off Build 74206: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — targeted error-message fix plus a hermetic test and a skip broadening for the live-GitHub flake.
What was reviewed:
fetch_from_github5xx now maps toGitHubIsDown(wasNPMIsDown); the sibling npm-registry path at create_command.rs:2440 correctly staysNPMIsDown.- New
GitHubIsDown | HTTPTooManyRequestshandler mirrors the adjacent 403/404 branches; 429 previously fell through to the generic else, so this is strictly a message improvement. - New 503/429 tests use a local TLS server +
GITHUB_API_DOMAINoverride (real env var, read at create_command.rs:2260), drain both pipes concurrently, and assert exit code last.
Extended reasoning...
Overview
Two files: src/runtime/cli/create_command.rs (+13/-1) and test/cli/install/bun-create.test.ts (+42/-6). The runtime change (a) remaps 499–599 in Example::fetch_from_github from crate::Error::NPMIsDown to crate::Error::GitHubIsDown, and (b) adds a GitHubIsDown | HTTPTooManyRequests arm to the caller's error match that prints a GitHub-specific message and crashes — same shape as the existing HTTPForbidden and GitHubRepositoryNotFound arms right above and below it. The test change renames isGithubRateLimited → isGithubUnavailable, adds detection for the new message so the three live-GitHub tests skip on 5xx (as they already did on 403), and adds two hermetic tests (503, 429) that spin up a local Bun.serve with the harness tls cert and point GITHUB_API_DOMAIN at it.
Security risks
None. This is CLI error-message plumbing on a failure path; no auth, crypto, permission, or parsing changes. The test sets NODE_TLS_REJECT_UNAUTHORIZED=0 only in the spawned child's env against a localhost self-signed server — an established pattern in the suite.
Level of scrutiny
Low. The runtime change is a one-line enum swap plus a copy-pasted error-branch that follows its neighbors exactly (node.end(); progress.refresh(); pretty_error!(...); Global::crash();). The user-facing effect is only a better error message when GitHub returns 5xx/429 during bun create. I confirmed GitHubIsDown already exists in src/runtime/error.rs (used by upgrade_command.rs), that GITHUB_API_DOMAIN is actually consumed in fetch_from_github, and that the remaining NPMIsDown at create_command.rs:2440 belongs to the npm-registry example path so is correctly left alone.
Other factors
The new tests are hermetic (no live GitHub), use port: 0, using/await using for cleanup, drain stdout/stderr/exited concurrently, assert stderr contents before the exit code, and spread bunEnv. The skip-on-unavailable helper is a strict superset of the old behavior. No prior human reviews or unresolved comments on the thread. The bug hunter found nothing.
There was a problem hiding this comment.
LGTM — corrects a copy-paste bug where GitHub 5xx surfaced as NPMIsDown, and folds 429 into the rate-limit hint.
What was reviewed:
- Verified
GitHubIsDownexists insrc/runtime/error.rsand the remainingNPMIsDownat create_command.rs:2445 is the actual npm package.json fetch path, so correctly left alone. - Confirmed
GITHUB_API_DOMAINis read byfetch_from_github(create_command.rs:2265), so the new hermetic test's local-server override actually routes there. - Checked
isGithubUnavailablesubstrings match both new error messages (403/429 share "GitHub is rate limiting", 5xx has "GitHub returned a server error").
Extended reasoning...
Overview
Two-file change to bun create <github-url> error handling. Example::fetch_from_github was mapping 499-599 to crate::Error::NPMIsDown — a copy-paste from the adjacent npm fetch path — which surfaced to users as "An internal error occurred (NPMIsDown)" during the api.github.com outage on 2026-07-16. The PR:
- Maps GitHub 5xx to a new
GitHubIsDownarm with a dedicated user-facing message naming GitHub and the template. - Folds
HTTPTooManyRequests(429) into the existing 403 rate-limit branch, interpolating the status code so theGITHUB_ACCESS_TOKENremedy is shown for both. - Renames the test skip helper
isGithubRateLimited→isGithubUnavailableand extends it to also skip live-GitHub tests on 5xx, so CI doesn't go red during GitHub outages. - Adds a hermetic parametrized test that points
GITHUB_API_DOMAINat a localBun.servereturning 503/429/403 and asserts the exact per-status error text, that the skip helper recognizes each, that neitherNPMIsDownnor the generic internal-error string appears, and that the exit code is 1.
Security risks
None. This is CLI error-message routing; no auth, crypto, path handling, or untrusted-input parsing changes. The test sets NODE_TLS_REJECT_UNAUTHORIZED=0 only in the spawned child's env against a localhost self-signed server, which is standard harness practice.
Level of scrutiny
Low. A wrong-enum-variant fix in a CLI error path plus an additive error branch — no control-flow changes to the success path, no new allocations or lifetimes. The status-code match arm being changed already existed; only the returned variant name changes. Verified the sibling NPMIsDown at line 2445 is genuinely the npm fetch and correctly untouched, and that upgrade_command.rs already uses GitHubIsDown for the same pattern so this brings create into line.
Other factors
- The hermetic test follows harness conventions:
usingfor the server,await usingfor the process, concurrent pipe drain viaPromise.all, exit-code asserted last,port: 0. - The skip helper now matches on the shared substring "GitHub is rate limiting" rather than the literal "GitHub returned 403", so it correctly covers both 403 and 429 without duplication.
bstr::BStr::new(template)in the newpretty_error!matches howtemplate(a&[u8]) is formatted elsewhere in the file.
242b4c4 to
2d0d383
Compare
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughChangesGitHub error handling
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/cli/create_command.rs`:
- Line 2342: Update the status-code match range in the relevant error handling
around the GitHub response mapping to begin at 500 instead of 499, preserving
the existing upper bound of 599 and GitHubIsDown error behavior for true 5xx
responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 187e21a1-99cb-4aef-afa4-a62b7f9fb124
📒 Files selected for processing (2)
src/runtime/cli/create_command.rstest/cli/install/bun-create.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/install/bun-create.test.ts`:
- Around line 113-121: Update githubUnavailableReason to reference
GITHUB_ACCESS_TOKEN, matching the token variable required by the CLI’s
rate-limit message; leave the other unavailable-reason handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c55a82d3-1411-4dce-a6cc-c0f2682576ae
📒 Files selected for processing (1)
test/cli/install/bun-create.test.ts
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
While this fixes the specific issue, this does not answer why the test SIGABRT'd.
There was a problem hiding this comment.
LGTM — the earlier nit about isGithubUnavailable logging inside the hermetic assertions was addressed in 74127d6.
What was reviewed:
fetch_from_githubstatus mapping: 5xx now →GitHubIsDown(matchesupgrade_command.rs); the499..=599range is pre-existing and consistent with siblings.- New 429/
GitHubIsDownarms at theGithubRepositorycall site mirror the neighboringGitHubRepositoryNotFoundarm (node.end()/progress.refresh()/pretty_error!/Global::crash(), samebstr::BStr::new(template)). - Hermetic tests:
GITHUB_API_DOMAINis read atcreate_command.rs:2265andreject_unauthorizedat :2334, so the local-TLS override reaches the code under test;tlsis exported from harness. - Skip helper split into pure
githubUnavailableReason+ loggingisGithubUnavailable; live-GitHub tests now skip on 5xx/429 as well as 403.
Extended reasoning...
Overview
Two files. In src/runtime/cli/create_command.rs: (1) the fetch_from_github status match swaps NPMIsDown → GitHubIsDown for the pre-existing 499..=599 arm — a one-token fix for a copy-paste from the npm-registry path just below; (2) at the ExampleTag::GithubRepository call site, HTTPTooManyRequests is folded into the existing 403 rate-limit arm (status code interpolated so the message still names 403 vs 429), and a new GitHubIsDown arm prints a GitHub-specific server-error message. Both new arms copy the exact node.end() → progress.refresh() → pretty_error! → Global::crash() sequence and the bstr::BStr::new(template) formatting used by the neighboring GitHubRepositoryNotFound arm. In test/cli/install/bun-create.test.ts: three hermetic tests point GITHUB_API_DOMAIN at a local TLS Bun.serve returning 503/429/403 and assert the expected message (and absence of NPMIsDown/An internal error occurred); the pre-existing skip helper is renamed isGithubUnavailable and extended to cover 5xx/429, with the string-match logic split into a pure githubUnavailableReason predicate.
Security risks
None. This is error-message wording and CI test resilience. No new inputs are parsed, no auth/crypto/permission code touched, no memory or lifetime changes. The NODE_TLS_REJECT_UNAUTHORIZED=0 set in the test env only applies to the spawned test subprocess talking to the local self-signed test server.
Level of scrutiny
Low. The Rust change is a targeted error-variant swap plus two new error-message arms that are structurally identical to the arm directly below them; the success path is untouched. GitHubIsDown already exists in src/runtime/error.rs and is what upgrade_command.rs uses for the same status range, so this brings bun create in line with bun upgrade. The test changes follow harness conventions (port: 0, using for server/proc, {...env, ...}, concurrent pipe drain, exit-code asserted last) and are hermetic — they don't depend on GitHub's state.
Other factors
I verified the env plumbing the hermetic tests rely on: GITHUB_API_DOMAIN is read at create_command.rs:2265 and env_loader.get_tls_reject_unauthorized() at :2334, so the local-server override and self-signed-cert bypass both reach the code under test. My previous inline nit (the console.warn side effect leaking "Skipping:" lines from the hermetic assertions) was addressed in 74127d6 by splitting the predicate. Both CodeRabbit threads (the 499 lower bound and GITHUB_TOKEN naming) were answered — the range is pre-existing and shared with three sibling sites, and fetch_from_github reads GITHUB_TOKEN first (:2290-2294) — and are resolved. No outstanding reviewer comments remain.
|
The SIGABRT is
The stack clusters match: I opened #34425 for that: it makes This PR is scoped to the 5xx/429 error-message path (the |
|
fix merge conflicts pls |
…tests on it `bun create <github-url>` fetches https://api.github.com/repos/{owner}/{repo}/tarball. When GitHub returns a 5xx, `fetch_from_github` mapped that to the `NPMIsDown` error variant (a copy/paste of the npm fetch path that predates the Rust port), which surfaces to the user as "An internal error occurred (NPMIsDown)". During the api.github.com outage on 2026-07-16 the three GitHub-hitting tests in bun-create.test.ts failed with that message on every retry across multiple CI lanes (builds 74144, 74149, 74151-74154 and counting). Fix the GitHub 5xx path to return `GitHubIsDown` (the variant `bun upgrade` already uses for the same case) and print the same style of user-facing message the existing 403 handler prints, naming GitHub instead of falling through to the generic internal-error text. Cover the new message with a hermetic test that points GITHUB_API_DOMAIN at a local TLS server returning 503/429. On the test side, extend the existing rate-limit skip helper so the three live-GitHub tests also skip (rather than fail) when GitHub is serving 5xx, mirroring what #29956 already did for 403.
…x only 429 is a rate-limit response and the actionable remedy is the same GITHUB_ACCESS_TOKEN hint already shown for 403, so fold it into that arm (with the status code interpolated) instead of the 5xx server-error arm. The hermetic test now asserts the per-status message and that the skip helper recognizes all three.
…ests stop printing 'Skipping:'
99aa789 to
818ceee
Compare
There was a problem hiding this comment.
LGTM — straightforward error-variant correction with hermetic test coverage; my earlier nit about the Skipping: side effect was addressed in a95dd67.
What was reviewed:
fetch_from_github5xx now maps toGitHubIsDown(matchingupgrade_command.rs); the499..=599range is pre-existing and consistent with sibling sites.- New 429/
GitHubIsDownarms at the call site follow the exactnode.end(); progress.refresh(); pretty_error!; Global::crash()pattern of the neighbouringHTTPForbidden/GitHubRepositoryNotFoundarms. - Hermetic tests use a local TLS server via
GITHUB_API_DOMAIN(same pattern as the #34425 split-body test), drain pipes concurrently, and pin the skip predicate to the Rust error text via the puregithubUnavailableReason— no moreSkipping:noise.
Extended reasoning...
Overview
Two files: src/runtime/cli/create_command.rs swaps the 5xx error variant in fetch_from_github from NPMIsDown to GitHubIsDown (one token) and adds two arms at the GithubRepository call site — folding HTTPTooManyRequests into the existing 403 rate-limit message (with the status code interpolated) and a new GitHubIsDown arm with a GitHub-specific server-error message. test/cli/install/bun-create.test.ts renames the skip helper to isGithubUnavailable, splits it into a pure predicate + logging wrapper (per my earlier inline comment), extends it to cover 5xx, and adds three hermetic test.each-style tests that point GITHUB_API_DOMAIN at a local TLS server returning 503/429/403.
Security risks
None. This is CLI error-message routing and test infrastructure. No auth, crypto, permissions, or user-input parsing changes. NODE_TLS_REJECT_UNAUTHORIZED=0 is scoped to the spawned subprocess against a localhost self-signed server — the same pattern already used by bun-upgrade.test.ts and the split-body test from #34425.
Level of scrutiny
Low. The Rust change is a one-token variant swap plus two error-handling arms that copy the exact structure of the adjacent HTTPForbidden and GitHubRepositoryNotFound arms (node.end(); progress.refresh(); pretty_error!(...); Global::crash();). GitHubIsDown is the variant upgrade_command.rs:313/:719 already use for the identical case, so this brings bun create in line with bun upgrade. The npm fetch path below still uses NPMIsDown, which is correct there. The 499..=599 range is pre-existing and shared across three other sites — coderabbit raised then withdrew that.
Other factors
- All three prior inline threads are resolved: my
console.warnside-effect nit (fixed by splitting outgithubUnavailableReason), coderabbit's499..=599(pre-existing, out of scope), and coderabbit'sGITHUB_TOKENwording (fetch_from_githubreads it first). - The hermetic tests are well-formed:
using/await usingfor cleanup,port: 0, concurrentPromise.allon stdout/stderr/exited, exit-code assertion last, and they assert both the positive (toContain(expected)) and negative (not.toContain("NPMIsDown"),not.toContain("An internal error occurred")) contracts. ThegithubUnavailableReason(err)assertion pins the skip predicate to the actual Rust output so a future wording change can't silently stop the live tests from skipping. - dylan-conway's only request was to rebase for a merge conflict, which was done (the
tlsimport from #34425 was the only overlap). Gate evidence in the PR body shows the 503/429 tests fail on main and pass with the fix; 18 tests pass post-rebase.
3 participants
Problem
test/cli/install/bun-create.test.tswent red in CI on every retry on multiple lanes (builds 74144, 74149, 74151, 74152, 74153, 74154) with:The three affected tests spawn
bun create https://github.com/dylan-conway/create-test, which fetcheshttps://api.github.com/repos/dylan-conway/create-test/tarball. During the api.github.com outage on 2026-07-16 that endpoint (and/zen,/repos/..., etc.) started serving 503 Unicorn pages for GET requests, andfetch_from_githubmaps every 5xx from GitHub tocrate::Error::NPMIsDown. That variant has been there since the original Zig implementation (it was copy/pasted from the npm-registry fetch path just below it), so a GitHub outage surfaces to users as "NPMIsDown" and to CI as a hard failure. The 429 path falls through to the same generic "internal error" message.#29956 previously made these tests skip on GitHub 403 (rate limit), but 5xx/429 still fell through.
Fix
src/runtime/cli/create_command.rs: infetch_from_github, map 5xx toGitHubIsDown(the variantbun upgradealready uses for the same case) instead ofNPMIsDown. At theGithubRepositorycall site:HTTPTooManyRequestsinto the existing 403 arm so 429 gets the same rate-limit message with theGITHUB_ACCESS_TOKENremedy (the status code is interpolated so the message still names which one was returned);GitHubIsDownarm that prints a server-error message naming GitHub and the template instead of falling through to the generic "An internal error occurred (...)".The npm-registry
fetchpath below still usesNPMIsDown, which is correct there.test/cli/install/bun-create.test.ts: add three hermetic tests that pointGITHUB_API_DOMAINat a local TLS server returning 503/429/403 and assert the expected message for each (and thatNPMIsDown/An internal error occurredis not printed). These fail on main for 503 and 429 and pass with the fix regardless of whether GitHub is up.isGithubUnavailable) so the three live-GitHub tests also skip on 5xx and 429, the same way they already skip on 403.Verification
There is no single culprit PR for the test break itself; the live-GitHub dependency predates #29956 and the
NPMIsDownlabel predates the Rust port (#30412). The outage is the trigger.[review] gate passed · iteration 2 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 2 rejected · iteration 2
evidence per changed file