Repository navigation
fix: propagate diff timeouts and declared diff errors to clients - #72
Merged
Merged
Conversation
When a diff operation exhausted the service's 8 s observation budget, the raw context.DeadlineExceeded escaped the service, so the server projected it as a generic internal error and clients could not explain it. Exported workingdiff methods now convert expiry of the service's own budget into limit_exceeded with limit "deadline", the shape ADR 0023 declares. Cancellation or expiry of the caller's context is returned unchanged, so cancellation remains cancellation. The budget is a service field so tests can exhaust it deterministically. Fixes #69
projectDiffError built a protocol.DiffError from every API error and reported anything that failed DiffError.Validate as "daemon returned malformed diff error". That hid two classes of legitimate replies: - generic codes diff operations declare, such as internal and invalid_request, plus the common unauthorized-style rejections; - diff errors with required details (limit_exceeded, unsupported_repository, capacity_exceeded), because the transport decodes declared details into TypedDetails while the projection read only the untyped Details map. Only diff error codes are now projected as DiffError, with typed details flattened into the DiffError details map; every other declared code goes through the ordinary ServerError projection. Malformed diff details are still rejected. Adds DiffErrorCode.Valid alongside the existing ScratchpadErrorCode.Valid. Fixes #70
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.
Fixes #69. Fixes #70.
Follow-up to #71. When a diff failed, clients saw "daemon returned malformed diff error" instead of the actual failure. Two bugs on either side of the wire caused this; each has its own commit.
#69: deadline expiry escaped as a generic
internalerrorffbeaf2e fix(workingdiff): report observation deadline as a typed limitcontext.DeadlineExceededcame back from roughly 48 sites ininternal/workingdiff. The server mapped it tointernal.workingdiffmethod (Observe,ListTargets,ObserveTarget,ReadFile,ReadLineRange,ReadFileForAnnotation) now defersprojectDeadline. That converts expiry of the service's own budget intolimit_exceededwithdetails.limit: "deadline", the shape ADR 0023 already declares andDiffError.Validatealready accepts.Servicefield (defaultobservationBudget), so tests can exhaust it deterministically.limit_exceededwithlimit: "deadline"was already declared for every diff operation, so the OpenAPI, Swift sources and protocol version are unchanged.#70: the Go client reported declared errors as malformed
f93d5ab8 fix(api): project declared diff errors without reporting them malformedprojectDiffErrorbuilt aprotocol.DiffErrorfrom every API error and reported any validation failure as malformed. That hid two kinds of legitimate replies:internal,invalid_request, and the commonunauthorized-style rejections), as described in the issue.limit_exceeded,unsupported_repository,capacity_exceeded). This is not in the issue. The transport decodes declared details intoTypedDetails, but the projection read only the untypedDetailsmap, so validation always failed. Without this fix, the Diff timeouts reach clients as generic internal errors #69 deadline error would still have reached Go clients as "malformed".The fix:
*protocol.DiffError. Typed details are flattened into its details map through their JSON field names.ServerErrorprojection. Undeclared codes are already rejected by the transport.DiffErrorCode.Valid(), matchingScratchpadErrorCode.Valid().DiffError.Validatenow uses it. This is Go-only; there is no wire change.Validation
workingdiff: with an exhausted budget, list targets, observe target, observe working tree, and read committed file each return the exact typed deadline error, which passesDiffError.Validate. With the projection disabled, all four return the rawcontext.deadlineExceededError.limit_exceededbody.unavailable,internal,invalid_requestandunauthorized. 6 of these 8 were reported as malformed before the fix. A case with invalid details is still rejected.gofmt -l .,go build ./...,go vet ./...,go test ./...,go test -race ./internal/workingdiff/ ./internal/server/ ./api/....Client impact
internal.limit_exceededtext for timeouts.