ci: keep Bundle Size job green on transient GitHub comment failures - #1795
Conversation
The size measurement and job summary had already succeeded on PR #1789 (run 32050847506) when the PR comment write got a 503 during a GitHub incident and failed the whole lane. --post-comment now retries 5xx / 429 / network errors (4 attempts, 1s/2s/4s backoff) on both the list and write calls. If it still fails, it prints a ::warning::, appends a note to $GITHUB_STEP_SUMMARY, and exits 0. Other 4xx (bad token, missing permissions) stay fatal.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
Request changes: The retry wrapper blindly reissues the initial POST after a network/5xx failure. That request may have succeeded server-side before its response was lost, so retries can create duplicate marker comments. Make creation idempotent or reconcile after an uncertain create rather than repeating POST unchanged. The exact head also fails required Fallow Code Quality because |
…egressions Retry now wraps the whole list -> write cycle rather than each request, so a create whose response was lost (network error / 5xx) is re-listed on the next attempt and turned into a PATCH of the marker comment instead of a duplicate POST. Splits the retry/classify helpers under the fallow complexity gate. Adds scripts/__tests__/size-report-post-comment.test.ts (unit-core): spawns the real script against a stubbed fetch and pins uncertain-create reconciliation, transient exhaustion (warn + exit 0), and fatal 4xx (nonzero, no retry). SIZE_REPORT_RETRY_BASE_MS lets the tests skip real backoff.
|
Addressed in 303c47e:
🤖 Addressed by Claude Code |
|
Re-reviewed exact head 303c47e. The prior retry blocker is resolved: retries now re-list before writing, the lost-POST regression proves GET→POST→GET→PATCH, fatal/transient outcomes are covered, and Fallow is green. One remaining scope blocker: commit 2baae3 also changes 45 byte-exact Maestro conformance corpus YAML files only through quote/newline formatter churn. Those files are unrelated to the size-comment fix and should be reverted from this PR (PR #1803 addresses the formatter sweep itself). iOS Smoke is still pending; other exact-head checks pass. |
thymikee
added
the
ready-for-human
|
1 participant
Summary
On PR #1789 (run 32050847506) the size measurement and job summary completed, then
--post-commentgot a503 No server is currently availablefrom the GitHub comments API during an incident and the wholeBundle Sizelane went red. The lane's real job had already succeeded; only the cosmetic comment write failed.scripts/size-report.mjs--post-commentis now best-effort against transient GitHub failures, and idempotent under retry:retryTransient(() => syncGitHubComment(...))), 4 attempts with 1s/2s/4s backoff on 5xx / 429 / network error. Because every attempt re-lists first, a create whose response was lost but landed server-side is reconciled into aPATCHof the marker comment on the next attempt — no duplicatePOST(addresses the review comment).TransientGitHubErroris caught at the top:::warning::annotation, one-line note appended to$GITHUB_STEP_SUMMARY, exit 0.assertGitHubWriteResponseremoved; it guarded only this comment write, the sole GitHub write in the file.SIZE_REPORT_RETRY_BASE_MSenv override so the regression tests do not sleep through real backoff.size.ymlneeds no change: it already appends the full report to$GITHUB_STEP_SUMMARYbefore the comment step, so the numbers were never lost — only the comment.Validation
New
scripts/__tests__/size-report-post-comment.test.ts(registered in theunit-coreproject) spawns the real script with a--imported fetch stub driven by a per-call response script, and asserts on exit code, stdout/stderr,$GITHUB_STEP_SUMMARY, and the exact sequence of HTTP calls:GET, POST(net error), GET(marker now present), PATCH /42— no secondPOST503×4→::warning::on stdout, note in step summary, exit 0, one list call per attemptGET 200, POST 401→ nonzero exit,Failed to create PR comment: 401, no::warning::, no retryPlanted red evidence (script swapped, tests run, script restored):
main's script → reconciliation ✗ (throws on the net error), exhaustion ✗ (exit 1 on 503), fatal-4xx ✓ (guard: fatality preserved).POST, POSTvs expectedPOST, PATCH /42. (The exhaustion test on that head only timed out at 5s because it lacked the backoff override — harness artifact, not behavioral evidence.)Local gates on the final head:
oxfmt --check,oxlint --deny-warnings,fallow audit --base 302019563(✓ no issues; the earlier CRAP/cyclomatic finding onattemptGitHubRequestis gone — retry/classify are split intoretryTransient/backoffOrRethrowandgithubRequest/fetchOrTransient/githubStatusError),vitest run --project unit-core scripts/__tests__/size-report-post-comment.test.ts(3/3), and standalonetsc --stricton the test file. The earlierAnalyze (python)failure was CodeQL init dying on the same GitHub 503 outage, not this change.