Skip to content

feat(artifacts): stream file downloads; no overall timeout on byte transfers - #115

Open
ysyneu wants to merge 2 commits into
feat/artifact-downloadfrom
artifact-download
Open

ysyneu wants to merge 2 commits into
feat/artifact-downloadfrom
artifact-download

Conversation

@ysyneu

@ysyneu ysyneu commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Problem

Artifacts.ReadStream (GET /safari/artifact/stream) was generated on the typed JSON path. That path is wrong for a file whose type and size are arbitrary:

  • JSON files are lost. processResponse decodes a JSON body as the response envelope, so Raw comes back empty. A JSON file shaped like an error envelope is reported as an API error.
  • Files over 10MB are truncated without an error. Raw is read through io.LimitReader(maxResponseBodySize).
  • Slow transfers time out. http.Client.Timeout (30s by default) also bounds reading the body, so any transfer longer than 30s fails, even a healthy one. The same applies to uploads and to Sessions.Export.

Change

  • The generator skips artifact-read-stream. A hand-written Artifacts.Download(ctx, token) returns the live body as an io.ReadCloser.
    • Content-Type, Content-Length and Content-Disposition are available on resp.Header.
    • A non-2xx status yields the typed *ErrorResponse.
  • Timeouts by API class.
    • JSON calls keep the client Timeout.
    • Byte transfers use transferClient(): the configured client with Timeout = 0, sharing its Transport (so custom dialers and retry transports still apply). Callers bound these with ctx. Byte transfers are file upload, file download, session export and session subscribe.
  • Sessions.Subscribe already copied the client without its timeout inline. It now uses the same helper. openStream takes a method, so GET streams use it too.

Breaking: Artifacts.ReadStream and ArtifactsReadStreamRequest are removed. Use Artifacts.Download.

Verification

  • go test ./... passes.
  • golangci-lint v2.13.0 reports 0 issues.
  • New tests cover:
    • a JSON file shaped like an error envelope comes back byte-for-byte;
    • a 10MB+1KB body sent with a 150ms mid-body pause is read in full by a client with WithTimeout(50ms);
    • a 410 error envelope becomes *ErrorResponse with a nil body.

…ansfers

Artifacts.ReadStream went through the typed JSON path, which is wrong for a
file of arbitrary type and size:
- a JSON file was decoded as the response envelope, so Raw came back empty
  (or, if it looked like an error envelope, as an API error);
- Raw is capped at 10MB, so larger files were silently truncated;
- http.Client.Timeout (30s by default) also bounds reading the body.

The generator now skips artifact-read-stream, and the hand-written
Artifacts.Download(ctx, token) returns the live body for the caller to copy.

Byte transfers (file upload, file download, session export, session
subscribe) now share one transferClient: the configured client without an
overall timeout, so a long but healthy transfer is not cut off. Callers bound
them with ctx. JSON calls keep the client timeout.
…t scope

sanitizeURL only redacted app_key, so the t token of an artifact download (valid 5 minutes) reached request logs. WithTimeout/WithHTTPClient now state that their timeout bounds JSON calls only; byte transfers are bounded by ctx.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant