Skip to content

The presigned PUT is the fourth step of one user action and the only one that exits 1 — and it leads with the HTTP mechanism #388

Description

@ZacxDev

Split out of the #386 audit (the #374 fix). Pre-existing, deliberately not fixed there so the classification change stayed reviewable on its own.

The gap

IngestAssetFullRes is three HTTP calls, not two: mint → PUT the bytes → persist. #386 unified steps 1 and 3 (both listingOpIngest, both "the server rejected the image-upload request (400): … — no listing was changed; check the image and retry", both exit 2). Step 2 does not go through listingError at all — putPresigned (internal/appapi/listing.go) builds its own error.

Measured against an httptest server returning 400 EntityTooLarge on the presigned PUT:

PUT-step error: image upload PUT failed (HTTP 400): EntityTooLarge
errors.Is(err, civitai.ErrBadRequest) = false
errors.Is(err, civitai.ErrNetwork)    = false
errors.Is(err, civitai.ErrNotFound)   = false

Untagged, so exitCode's default: arm gives exit 1, where the same user action exits 2 if step 1 or step 3 400s.

Why it matters

  1. One user action, two exit codes, decided by which of three steps the storage backend refused. A script branching on 2 for "your input was bad" misses the oversize-cover case entirely.
  2. The message leads with the mechanism. "image upload PUT failed" names an HTTP verb the author never typed. four errors that name no next command, in a tool whose house style always does — including one raw Cobra message and two leaked procedure names #363 removed exactly this shape from the tRPC path (appListings.setIcon rejected the request) because it reads as the CLI being broken rather than the file being wrong.
  3. The author loses the diagnostics. attachRejectionAdvice short-circuits on this error, so the "what was sent" block (decoded dimensions, byte count, the bound that applied) never prints — and this is the step where an oversize cover actually lands, since covers are not rescaled.

Suggested fix

Route the PUT failure through the same classification: tag it civitai.ErrBadRequest for a 4xx (and ErrNetwork for 5xx), and word it as an ingest — "the server rejected the image-upload request (HTTP 400): EntityTooLarge — no listing was changed; check the image and retry" — so all four steps of the one action tell one story. TestIngestFullResTellsOneStoryAtEitherStep in internal/appapi/listing_op_test.go is the test to extend to a third step; the listingRoute ledger there does not cover the PUT because its target is a presigned URL on another host, so that seam needs its own case.

Already shipped in #386

README.md's Troubleshooting index carries a row for image upload PUT failed stating the exit-1 inconsistency explicitly, so the published contract is true today rather than silently wrong. That row should be folded back into the ingest row when this is fixed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions