Repository navigation
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Generated files now come from gofmt-clean emitters (column alignment in const blocks, struct types, and composite literals; one struct-make closing-brace indent). The handwritten checksum/hash_test.go loses its trailing blank line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Murderlon
left a comment
Murderlon
left a comment
There was a problem hiding this comment.
Murderlon gave this AI agent permission to post this review in their name.
Standards
Structural judgement: url_storage_generated.go has 1,778 lines. It combines storage, upload control, retry policy, and HTTP error recording. These responsibilities obscure the storage boundary. Split the generator output into focused files.
Spec
Five reproduced failures affect parallel uploads, draft completion, custom retry policy, and cancellation. See the inline comments. Apply generated-code fixes in the API2 generator.
Tests: go test -race ./... passed. go test -tags api2devdock ./examples/... compiled the examples. Five temporary reproduction tests failed. git diff --check passed.
| go func(partInput generatedTusParallelPartInput) { | ||
| defer workers.Done() | ||
| result := parallelClient.uploadParallelPartWithURLStorage(options, partInput) |
There was a problem hiding this comment.
[P1] Initialize capabilities before starting workers. Each worker calls CreateUpload on the same client. With a new client, these calls run UpdateCapabilities concurrently and write to the shared Capabilities field. A local two-part upload test reports this data race. Query capabilities once before starting workers. Give the workers the completed capability data.
| if headerName, value, ok := protocolUploadCompleteHeader(c.ProtocolVersion, true); ok { | ||
| headers[headerName] = value | ||
| } |
There was a problem hiding this comment.
[P1] Do not mark partial creation data as complete. Passing true overrides the completion value that uploadChunkImpl already calculates. With draft-05, a creation request with 2 of 5 bytes sends Upload-Complete: ?1. This marks an unfinished upload as complete. Remove this override so the stream calculation controls the header.
| parallelCtx, cancelParallelUploads := generatedTusParallelUploadContext(options.Context) | ||
| defer cancelParallelUploads() | ||
| parallelClient := uploadClient.WithContext(parallelCtx) |
There was a problem hiding this comment.
[P1] Preserve the client context in parallel workers. When options.Context is nil, this helper uses context.Background() and replaces the context set by Client.WithContext. A local test with an already cancelled client still sends both partial POST and PATCH requests. Derive the worker context from uploadClient.ctx so cancellation also stops partial uploads.
| if retryAttempt >= len(retryDelays) || !generatedTusShouldRetryStatus(statusCode) { | ||
| return false | ||
| } | ||
| if onShouldRetry != nil { | ||
| return onShouldRetry(err, retryAttempt) | ||
| } |
There was a problem hiding this comment.
[P2] Run the custom retry decision before the default policy. The contract requires custom-callback-before-default-decision. This guard rejects HTTP 400 before OnShouldRetry runs. A callback that returns true cannot override that decision. Check delay exhaustion first. Use the callback when it exists. Apply the default status policy only when no callback exists.
| delay := retryDelays[retryAttempt] | ||
| if delay > 0 { | ||
| time.Sleep(delay) | ||
| } |
There was a problem hiding this comment.
[P2] Let cancellation stop retry delays. Both retry loops use time.Sleep, so context cancellation cannot stop a pending retry. A local test cancels after 20 ms but returns after the full 600 ms delay. The abort contract requires cancellation of pending retry timers. Use one context-aware wait helper for termination and upload retries.
3 participants
Experimental Status
Experimental work: this PR is part of an exploratory contract/generated SDK effort, and is intentionally kept as a Draft while the approach is validated.
Why
This adds generated islands and checked-in devdock examples for the API2-owned TUS protocol client generator in tus-go-client. The goal is to prove that a non-TypeScript TUS client can consume the same raw protocol + feature-layer contract as tus-js-client without duplicating endpoint/header/status knowledge in the language generator.
What
protocol_generated.gowith the defaultTus-Resumablewire version.protocol_contract_generated_test.gowith TUS wire operations, response shapes, and client feature metadata.NewClientwithout changing behavior.Client.CreateUploadandUploadStream.Writefrom generated contract facts.serverCapabilities.extensionNamesandserverCapabilities.protocolVersionsfrom the contract scenario and only adapts those facts intotusgo.ServerCapabilities.endpointUrl,uploadUrl,overridePatchMethod, fingerprint, content, and server capabilities from scenario JSON, then provesHEADoffset recovery followed byPOSTwithX-HTTP-Method-Override: PATCH.relative-to-creation-request-url, so stored URLs, hooks, and subsequent upload requests all use the resolved absolute upload URL.Upload-Concatheader, checks request body starts and absent finalUpload-Length, and proves publicUploadWithURLStoragewithParallelUploads.ParallelUploadsplusUploadDataDuringCreationfails before any HTTP request.Client.ProtocolVersionselects request headers, upload body content type, and upload-complete headers, then proves draft-05 creation-with-upload sendsUpload-Draft-Interop-Version,Upload-Complete, andapplication/partial-uploadwhile omittingTus-Resumable.inputSource.contentinto a temporary file, passes an opened file through publicUploadWithURLStorage, and captures the contract-declared source-open/success/source-close events plus POST/PATCH request facts.DetailedErrorruntime support and a generated recorder transport to prove both response-status and request-error create-upload failures through publicUploadWithURLStorage.Verification
go test ./...go test -tags api2devdock ./examples/...git diff --checktusAbortUploadtusAbortUploadAfterStoredUrltusOverridePatchMethodtusRelativeLocationResolutiontusParallelUploadConcattusRetryStateTransitionstusStartOptionValidationParallelUploadsWithUploadDataDuringCreationtusProtocolVersionSelectionDraft05CreationWithUploadtusNodePathInputSourcetusDetailedCreateResponseErrortusDetailedCreateRequestErrorapi2/bin/cli.ts contracts sdks --no-motd --target tus --platform go --sdk-root ../tus-go-client --compare-existingapi2/bin/cli.ts contracts sdks qa --no-motd --target tus --platform go --dry-runcore/bin/devdock.ts exec tstrun system/sdk_examples/go-tus-transloadit-assembly-upload.test.ts -vv --max-time-per-test 900Companion PRs (tus org only)