Repository navigation
🐛 fix(git): classify HTTP status codes from typed errors and status phrases; stabilise delete-watch envtest - #459
Merged
Merged
Conversation
The envtest spec deleted its sink as soon as the cache synced, but the manager attaches a controller's Watches handler asynchronously after that. A delete landing before registration is never replayed to the handler (late handlers only get synthetic Adds), so the spy saw zero closes and the spec timed out. Probe with throwaway sinks until one delete reaches the seam, then run the real assertion.
ClassifyExportError matched 401/503/429 as bare substrings, so digits in a random temp-dir path (a missing local remote) read as an auth failure and turned a transient clone error terminal. TestRunCleanupExport_ gitReleaseFailureIsRetried failed in CI whenever the dir name contained 401. Require a non-digit boundary around the code.
konih
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…hrases Bare-substring and digit-boundary matching still misread codes inside URLs, hostnames and hex mirror dirs (team401, git-503.example.com, 3fa401bc9e0d). Read the status from go-git's typed http.Err and from status phrases only (status code: N, returned error: N, HTTP N, N forbidden, (N)), classify 401/403 terminal and 429/5xx transient, and treat ErrAuthorizationFailed as auth. The git CLI's returned error: 403 is now terminal too. The delete-watch envtest now gates on an observed reconcile of the sink (workers start after the Watches handler is registered) instead of probe deletes, and no longer leaks an unused sink.
konih
changed the title
|
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
1 participant
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.
Two pre-existing flakes that kept main red (not #454 regressions).
Delete-watch envtest: the spec deleted its sink right after cache sync, but the controller's Watches handler is attached asynchronously afterwards; an earlier delete is never replayed, so closes stayed 0. Pre-♻️ refactor: delete the production-dead exported surface (kollect-xconsol §4 sweep 2) #454 fails 1/30, main 1/15. Fix: wait for an observed reconcile of the sink (workers start only after the handler is registered) before pooling and deleting. Mutation check: removing the Watches clause fails the real assertion after 30s.
TestRunCleanupExport_gitReleaseFailureIsRetried: ClassifyExportError matched 401/503/429 as bare substrings, so a random temp-dir name containing 401 turned a transient clone error terminal. Fix: read the status from go-git's typed http.Err and from status phrases only (never digit runs in URLs, hostnames or mirror dirs); 401/403 terminal, 429/5xx transient; ErrAuthorizationFailed is auth; the CLI's 'returned error: 403' is now terminal. Table test covers team401 URL, hex mirror dir, git-503 hostname and the existing rows.