Skip to content

fix(service): Return 400 for client-side upload stream errors#374

Merged
jan-auer merged 1 commit intomainfrom
worktree-client-stream-error
Mar 16, 2026
Merged

fix(service): Return 400 for client-side upload stream errors#374
jan-auer merged 1 commit intomainfrom
worktree-client-stream-error

Conversation

@jan-auer
Copy link
Copy Markdown
Member

@jan-auer jan-auer commented Mar 13, 2026

When a client drops the connection mid-upload, the broken stream error was propagating as Error::Io and returning 500. The server is not at fault here — this should be a 400.

Dedicated stream type

Introduces ClientError and ClientStream as a distinct type from the outgoing PayloadStream. Previously, both incoming and outgoing streams were BoxStream<'static, io::Result<Bytes>>, making it impossible to distinguish a client-side failure from a backend I/O error at the type level.

ClientStream is now the type for all incoming request bodies. ClientError wraps the underlying axum/hyper body error and is Clone (backed by Arc).

Distinct status code

Adds Error::Client(ClientError) to the service error enum. The server maps this variant to 400 Bad Request, while all other I/O errors remain 500.

Unpacking client errors

Backends receive a ClientStream, convert it into a reqwest::Body or io::Reader for storage, and need to detect when a failure was caused by the client rather than the backend.

common::unpack_client_error walks the error source chain and checks two locations: the error itself (direct ClientError), and custom io::Error payloads (where StreamReader or reqwest/hyper may have wrapped it). All backends now call this after a failed put and return Error::Client when the source was the upload stream.

Generics

Several stream utilities were generalised over the stream and error type:

  • SizedPeek — previously hardcoded to PayloadStream; now avoids intermediate boxing
  • MeteredPayloadStream and Services::meter_stream — now a single generic implementation that works for both stream types and avoids an extra inner Box

When a client drops the connection mid-upload, the stream error was
propagating as Error::Io and returning 500. The server is not at
fault in this case.

Introduces ClientError and ClientStream as a distinct type from the
outgoing PayloadStream. All backends now call unpack_client_error()
after a failed put, which walks the error source chain (handling both
direct ClientError sources and those packed inside an io::Error) and
maps them to the new Error::Client variant. The server maps this to
400 Bad Request.

MeteredPayloadStream and meter_stream are now generic over the stream
and error type, avoiding an extra box and unifying metering for both
incoming and outgoing streams.

Co-Authored-By: Claude <noreply@anthropic.com>
@jan-auer jan-auer marked this pull request as ready for review March 13, 2026 16:30
@jan-auer jan-auer requested a review from a team as a code owner March 13, 2026 16:30
Comment thread objectstore-service/src/backend/common.rs
@jan-auer jan-auer merged commit 261fda3 into main Mar 16, 2026
21 checks passed
@jan-auer jan-auer deleted the worktree-client-stream-error branch March 16, 2026 07:31
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.

2 participants