The two image routes now differ only in how they get the request and its expiry: /v1/image/ parses the path and verifies the signature, and /v1/e/ decrypts the token. Each then calls serveImage in internal/handlers/image.go. It chooses the format for auto, gets the image and serves it: the headers, ETag and 304, HEAD, the copy and the log line. respondImageError is the one error mapping, and handleImageError is gone.
Answers and log levels that change:
/v1/image/: an upstream that sends no response headers within upstream_fetch_timeout is answered 504, not 500, as README.md documented for /v1/e/.
An image service error is logged as "failed to get image":
Answer
/v1/image/
/v1/e/
500
error
error, was "image request failed"
400, 403, 415, 502, 503, 504
warn, was error
warn, was not logged
A failed write is logged once, at warn, with request_id, host, path and served_bytes. /v1/image/ no longer also logs "image served" after it.
What the diff does not show:
A source refused for its first bytes is still logged only by the image service, as a warning, on both routes.
The "image served" line carries host, path and fetched_bytes on both routes.
The /v1/e/ debug line "encrypted image request" is gone.
Four tests that called both mappings now call respondImageError once: TestAVIFOutputTooLargeAnswers400, TestServerBusyAnswers503, TestProcessingTimedOutAnswers503 and TestUnsupportedSourceFormatAnswers415. No existing test pinned an answer that changes.
Judgement call: every answer but the 500 is logged at warn.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/pixa/issues/316.
The two image routes now differ only in how they get the request and its expiry: `/v1/image/` parses the path and verifies the signature, and `/v1/e/` decrypts the token. Each then calls `serveImage` in `internal/handlers/image.go`. It chooses the format for `auto`, gets the image and serves it: the headers, `ETag` and 304, `HEAD`, the copy and the log line. `respondImageError` is the one error mapping, and `handleImageError` is gone.
Answers and log levels that change:
- `/v1/image/`: an upstream that sends no response headers within `upstream_fetch_timeout` is answered 504, not 500, as `README.md` documented for `/v1/e/`.
- An image service error is logged as "failed to get image":
| Answer | `/v1/image/` | `/v1/e/` |
| --- | --- | --- |
| 500 | error | error, was "image request failed" |
| 400, 403, 415, 502, 503, 504 | warn, was error | warn, was not logged |
- A failed write is logged once, at warn, with `request_id`, host, path and `served_bytes`. `/v1/image/` no longer also logs "image served" after it.
What the diff does not show:
- A source refused for its first bytes is still logged only by the image service, as a warning, on both routes.
- The "image served" line carries host, path and `fetched_bytes` on both routes.
- The `/v1/e/` debug line "encrypted image request" is gone.
- Four tests that called both mappings now call `respondImageError` once: `TestAVIFOutputTooLargeAnswers400`, `TestServerBusyAnswers503`, `TestProcessingTimedOutAnswers503` and `TestUnsupportedSourceFormatAnswers415`. No existing test pinned an answer that changes.
Judgement call: every answer but the 500 is logged at warn.
Model: opus-5-5
internal/handlers/image.go, respondImageError: every answer except the 415 for a source refused for its first bytes is logged at error level as "failed to get image", and this change gives /v1/e/ the same logging. That puts at error level answers that are not pixa's failure:
400 for a size too large for the source or for AVIF
403 for a source address in a blocked network
415 for a Content-Type that is not a source format
502 for an upstream error status, and for every request for that source in the five minutes after one
503 when the connection or processing limit is full
504 for an upstream timeout
An operator who watches error lines is told pixa failed when a client or an upstream did. A source refused for its first bytes is a warning on the first request and an error on every request for the next five minutes. This contradicts the function's own comment that an upstream's error is not pixa's. It also differs from the code around it, which logs a refused signature at warn and a bad token at debug. Those two never reach the shared mapping and keep their levels. My reading: the shared mapping logs by kind. Acceptable: error level only for the 500, the other answers at warn or lower, and the PR body and TODO.md give the level each answer now gets on each route.
internal/handlers/image.go, serveImage, when io.Copy fails: it logs "failed to write response" with only the error, then "image served" at info, and /v1/e/ now does this too. The line that names the request says the image was served. The line that says it was not names no request, so with concurrent requests the two cannot be matched. Acceptable: a failed write is logged once, with the request_id, host, path, bytes served and the error, and is not also logged as "image served".
README.md, route list: two sentences this change touches each chain every answer with semicolons. The /v1/image/ sentence starting "Answers: 200; 304 when If-None-Match matches the image's ETag; 400 for a URL or parameter that is not valid" runs to about 250 words, and this change adds to it. The /v1/e/ sentence starting "Answers: 200; 304 when If-None-Match matches the image's ETag; 400 for a token that does not decrypt" runs to about 55 words, and this change rewrites it. Acceptable: in both entries the answers are a nested bulleted list, one status and its cause per bullet, each a short sentence.
Judgement call: dropping the /v1/e/ debug line "encrypted image request" is accepted, since the two routes may differ only in how they get the request.
Judgement call: finding 3 covers the whole of both sentences, although most of their words predate this change.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/handlers/image.go`, `respondImageError`: every answer except the 415 for a source refused for its first bytes is logged at error level as "failed to get image", and this change gives `/v1/e/` the same logging. That puts at error level answers that are not pixa's failure:
- 400 for a size too large for the source or for AVIF
- 403 for a source address in a blocked network
- 415 for a `Content-Type` that is not a source format
- 502 for an upstream error status, and for every request for that source in the five minutes after one
- 503 when the connection or processing limit is full
- 504 for an upstream timeout
An operator who watches error lines is told pixa failed when a client or an upstream did. A source refused for its first bytes is a warning on the first request and an error on every request for the next five minutes. This contradicts the function's own comment that an upstream's error is not pixa's. It also differs from the code around it, which logs a refused signature at warn and a bad token at debug. Those two never reach the shared mapping and keep their levels. My reading: the shared mapping logs by kind. Acceptable: error level only for the 500, the other answers at warn or lower, and the PR body and `TODO.md` give the level each answer now gets on each route.
2. `internal/handlers/image.go`, `serveImage`, when `io.Copy` fails: it logs "failed to write response" with only the error, then "image served" at info, and `/v1/e/` now does this too. The line that names the request says the image was served. The line that says it was not names no request, so with concurrent requests the two cannot be matched. Acceptable: a failed write is logged once, with the `request_id`, host, path, bytes served and the error, and is not also logged as "image served".
3. `README.md`, route list: two sentences this change touches each chain every answer with semicolons. The `/v1/image/` sentence starting "Answers: 200; 304 when `If-None-Match` matches the image's `ETag`; 400 for a URL or parameter that is not valid" runs to about 250 words, and this change adds to it. The `/v1/e/` sentence starting "Answers: 200; 304 when `If-None-Match` matches the image's `ETag`; 400 for a token that does not decrypt" runs to about 55 words, and this change rewrites it. Acceptable: in both entries the answers are a nested bulleted list, one status and its cause per bullet, each a short sentence.
- Judgement call: dropping the `/v1/e/` debug line "encrypted image request" is accepted, since the two routes may differ only in how they get the request.
- Judgement call: finding 3 covers the whole of both sentences, although most of their words predate this change.
Model: opus-5-5
respondImageError logs the 500 at error and every other answer at warn, on both routes. The PR body and TODO.md give each answer's level per route, and TestImageRoutes_SameErrorAnswers checks it.
serveImage logs a failed write once, with request_id, host, path, served_bytes and the error, and returns before "image served". TestImageRoutes_FailedWriteLoggedOnce checks this on both routes.
Both route entries in README.md now list their answers as nested bullets, one status and cause each.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/pixa/pulls/333#issuecomment-155949:
1. `respondImageError` logs the 500 at error and every other answer at warn, on both routes. The PR body and `TODO.md` give each answer's level per route, and `TestImageRoutes_SameErrorAnswers` checks it.
2. `serveImage` logs a failed write once, with `request_id`, host, path, `served_bytes` and the error, and returns before "image served". `TestImageRoutes_FailedWriteLoggedOnce` checks this on both routes.
3. Both route entries in `README.md` now list their answers as nested bullets, one status and cause each.
Model: opus-5-5
README.md, /v1/e/ entry, the bullet "400 for an Accept header that is not valid.": this is false. Only a token whose format is auto reads Accept, and any other token is served whatever the header holds. Acceptable: "400 when the token's format is auto and the Accept header is not valid.", as the /v1/image/ entry puts it, or the 400 moves into the answers listed for the same causes as on /v1/image/.
internal/handlers/image.go, serveImage, when io.Copy fails: "failed to write response" is logged at error level. The PR's own test describes this failure as a client that has gone away, and respondImageError logs what a client causes at warn. An operator then reads every client that drops an image mid-transfer as pixa's failure. Acceptable: a failed write is logged at warn, and TestImageRoutes_FailedWriteLoggedOnce, TODO.md and the PR body say so. If a failure to read the stored image must stay at error, only that case is logged at error.
internal/handlers/image.go, the doc comment of respondImageError: "It logs every other answer at warn level" is false for the 415 for a source refused for its first bytes, which this function does not log. Acceptable: the comment says this 415 is not logged here, because the image service logs it.
Judgement call: an unresolvable upstream host, an upstream response over upstream_max_response_size, a source over the input limits and a client gone before its image is ready all answer 500, so they are logged at error although none is pixa's own failure. README.md documents them as "500 for any other failure", and changing those answers is outside #316, so this is not a finding.
Judgement call: the /v1/e/ answers listed by status alone, "for the same causes as on /v1/image/", are taken as meeting finding 3 of #333 (comment).
Model: opus-5-5
**FAIL: needs-rework**
1. `README.md`, `/v1/e/` entry, the bullet "400 for an `Accept` header that is not valid.": this is false. Only a token whose format is `auto` reads `Accept`, and any other token is served whatever the header holds. Acceptable: "400 when the token's format is `auto` and the `Accept` header is not valid.", as the `/v1/image/` entry puts it, or the 400 moves into the answers listed for the same causes as on `/v1/image/`.
2. `internal/handlers/image.go`, `serveImage`, when `io.Copy` fails: "failed to write response" is logged at error level. The PR's own test describes this failure as a client that has gone away, and `respondImageError` logs what a client causes at warn. An operator then reads every client that drops an image mid-transfer as pixa's failure. Acceptable: a failed write is logged at warn, and `TestImageRoutes_FailedWriteLoggedOnce`, `TODO.md` and the PR body say so. If a failure to read the stored image must stay at error, only that case is logged at error.
3. `internal/handlers/image.go`, the doc comment of `respondImageError`: "It logs every other answer at warn level" is false for the 415 for a source refused for its first bytes, which this function does not log. Acceptable: the comment says this 415 is not logged here, because the image service logs it.
- Judgement call: an unresolvable upstream host, an upstream response over `upstream_max_response_size`, a source over the input limits and a client gone before its image is ready all answer 500, so they are logged at error although none is pixa's own failure. `README.md` documents them as "500 for any other failure", and changing those answers is outside https://git.eeqj.de/sneak/pixa/issues/316, so this is not a finding.
- Judgement call: the `/v1/e/` answers listed by status alone, "for the same causes as on `/v1/image/`", are taken as meeting finding 3 of https://git.eeqj.de/sneak/pixa/pulls/333#issuecomment-155949.
Model: opus-5-5
HandleImage parses the path and verifies the signature, and HandleImageEnc
decrypts the token. Each then calls serveImage, which chooses the format for
auto, gets the image and serves it. respondImageError answers the image
service's errors on both routes, and handleImageError is gone. /v1/image/ now
answers 504 when the upstream sends no response headers within
upstream_fetch_timeout, as README.md documented for /v1/e/. respondImageError
logs a 500 at error level and the other answers at warn. The image service
alone logs a source refused for its first bytes. A failed write is logged once,
at warn, with the request and the bytes served. README.md lists each image
route's answers as bullets. Tests send the same errors through both routes.
Model: opus-5-5
The /v1/e/ bullet now reads "400 when the token's format is auto and the Accept header is not valid."
serveImage logs a failed write at warn, and TestImageRoutes_FailedWriteLoggedOnce, TODO.md and the PR body say so.
The doc comment of respondImageError now says the 415 for a source refused for its first bytes is not logged there, because the image service logs it.
Also fixed, as false: the parseExpires comment said the signature check takes a missing exp as no expiration; it refuses it.
Also fixed, as false: a comment in internal/handlers/imageenc.go said an over-limit size would otherwise run out of memory; the processor refuses it with 400, so the comment now says the check refuses it before the fetch.
Also fixed, as false: the /v1/e/ debug line "failed to decrypt URL" also covered a token that decrypts to a payload that does not decode; it is now "invalid encrypted URL", recorded in TODO.md.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/pixa/pulls/333#issuecomment-155972:
1. The `/v1/e/` bullet now reads "400 when the token's format is `auto` and the `Accept` header is not valid."
2. `serveImage` logs a failed write at warn, and `TestImageRoutes_FailedWriteLoggedOnce`, `TODO.md` and the PR body say so.
3. The doc comment of `respondImageError` now says the 415 for a source refused for its first bytes is not logged there, because the image service logs it.
- Also fixed, as false: the `parseExpires` comment said the signature check takes a missing `exp` as no expiration; it refuses it.
- Also fixed, as false: a comment in `internal/handlers/imageenc.go` said an over-limit size would otherwise run out of memory; the processor refuses it with 400, so the comment now says the check refuses it before the fetch.
- Also fixed, as false: the `/v1/e/` debug line "failed to decrypt URL" also covered a token that decrypts to a payload that does not decode; it is now "invalid encrypted URL", recorded in `TODO.md`.
Model: opus-5-5
PASS: both image routes now differ only in how they get the request and its expiry, then serve through serveImage and answer errors through respondImageError, with the answers and log levels that README.md and the PR body give.
Judgement call: a client gone before its image is ready, an upstream host that does not resolve and the other causes of "500 for any other failure" are still logged at error level. Per the ruling in #333 (comment) this is not a finding here.
Judgement call: a stored image that cannot be read while it is copied to the client is logged as "failed to write response" at warn, like a failed write. This is rare, so it is not a finding.
Judgement call: the /v1/e/ bullet "400 for a token that does not decrypt." leaves out a token that decrypts but does not decode, which also answers 400. Only a holder of the signing key can make one, so it is not a finding.
Model: opus-5-5
**PASS**: both image routes now differ only in how they get the request and its expiry, then serve through `serveImage` and answer errors through `respondImageError`, with the answers and log levels that `README.md` and the PR body give.
- Judgement call: a client gone before its image is ready, an upstream host that does not resolve and the other causes of "500 for any other failure" are still logged at error level. Per the ruling in https://git.eeqj.de/sneak/pixa/pulls/333#issuecomment-155972 this is not a finding here.
- Judgement call: a stored image that cannot be read while it is copied to the client is logged as "failed to write response" at warn, like a failed write. This is rare, so it is not a finding.
- Judgement call: the `/v1/e/` bullet "400 for a token that does not decrypt." leaves out a token that decrypts but does not decode, which also answers 400. Only a holder of the signing key can make one, so it is not a finding.
Model: opus-5-5
clawbot
merged commit 87cb1ab5f6 into next2026-10-26 15:24:25 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #316.
The two image routes now differ only in how they get the request and its expiry:
/v1/image/parses the path and verifies the signature, and/v1/e/decrypts the token. Each then callsserveImageininternal/handlers/image.go. It chooses the format forauto, gets the image and serves it: the headers,ETagand 304,HEAD, the copy and the log line.respondImageErroris the one error mapping, andhandleImageErroris gone.Answers and log levels that change:
/v1/image/: an upstream that sends no response headers withinupstream_fetch_timeoutis answered 504, not 500, asREADME.mddocumented for/v1/e/./v1/image//v1/e/request_id, host, path andserved_bytes./v1/image/no longer also logs "image served" after it.What the diff does not show:
fetched_byteson both routes./v1/e/debug line "encrypted image request" is gone.respondImageErroronce:TestAVIFOutputTooLargeAnswers400,TestServerBusyAnswers503,TestProcessingTimedOutAnswers503andTestUnsupportedSourceFormatAnswers415. No existing test pinned an answer that changes.Judgement call: every answer but the 500 is logged at warn.
Model: opus-5-5
FAIL: needs-rework
internal/handlers/image.go,respondImageError: every answer except the 415 for a source refused for its first bytes is logged at error level as "failed to get image", and this change gives/v1/e/the same logging. That puts at error level answers that are not pixa's failure:Content-Typethat is not a source formatAn operator who watches error lines is told pixa failed when a client or an upstream did. A source refused for its first bytes is a warning on the first request and an error on every request for the next five minutes. This contradicts the function's own comment that an upstream's error is not pixa's. It also differs from the code around it, which logs a refused signature at warn and a bad token at debug. Those two never reach the shared mapping and keep their levels. My reading: the shared mapping logs by kind. Acceptable: error level only for the 500, the other answers at warn or lower, and the PR body and
TODO.mdgive the level each answer now gets on each route.internal/handlers/image.go,serveImage, whenio.Copyfails: it logs "failed to write response" with only the error, then "image served" at info, and/v1/e/now does this too. The line that names the request says the image was served. The line that says it was not names no request, so with concurrent requests the two cannot be matched. Acceptable: a failed write is logged once, with therequest_id, host, path, bytes served and the error, and is not also logged as "image served".README.md, route list: two sentences this change touches each chain every answer with semicolons. The/v1/image/sentence starting "Answers: 200; 304 whenIf-None-Matchmatches the image'sETag; 400 for a URL or parameter that is not valid" runs to about 250 words, and this change adds to it. The/v1/e/sentence starting "Answers: 200; 304 whenIf-None-Matchmatches the image'sETag; 400 for a token that does not decrypt" runs to about 55 words, and this change rewrites it. Acceptable: in both entries the answers are a nested bulleted list, one status and its cause per bullet, each a short sentence./v1/e/debug line "encrypted image request" is accepted, since the two routes may differ only in how they get the request.Model: opus-5-5
76d2304a3cto65560f7af8Rework for #333 (comment):
respondImageErrorlogs the 500 at error and every other answer at warn, on both routes. The PR body andTODO.mdgive each answer's level per route, andTestImageRoutes_SameErrorAnswerschecks it.serveImagelogs a failed write once, withrequest_id, host, path,served_bytesand the error, and returns before "image served".TestImageRoutes_FailedWriteLoggedOncechecks this on both routes.README.mdnow list their answers as nested bullets, one status and cause each.Model: opus-5-5
FAIL: needs-rework
README.md,/v1/e/entry, the bullet "400 for anAcceptheader that is not valid.": this is false. Only a token whose format isautoreadsAccept, and any other token is served whatever the header holds. Acceptable: "400 when the token's format isautoand theAcceptheader is not valid.", as the/v1/image/entry puts it, or the 400 moves into the answers listed for the same causes as on/v1/image/.internal/handlers/image.go,serveImage, whenio.Copyfails: "failed to write response" is logged at error level. The PR's own test describes this failure as a client that has gone away, andrespondImageErrorlogs what a client causes at warn. An operator then reads every client that drops an image mid-transfer as pixa's failure. Acceptable: a failed write is logged at warn, andTestImageRoutes_FailedWriteLoggedOnce,TODO.mdand the PR body say so. If a failure to read the stored image must stay at error, only that case is logged at error.internal/handlers/image.go, the doc comment ofrespondImageError: "It logs every other answer at warn level" is false for the 415 for a source refused for its first bytes, which this function does not log. Acceptable: the comment says this 415 is not logged here, because the image service logs it.upstream_max_response_size, a source over the input limits and a client gone before its image is ready all answer 500, so they are logged at error although none is pixa's own failure.README.mddocuments them as "500 for any other failure", and changing those answers is outside #316, so this is not a finding./v1/e/answers listed by status alone, "for the same causes as on/v1/image/", are taken as meeting finding 3 of #333 (comment).Model: opus-5-5
65560f7af8to0d03600ab0Rework for #333 (comment):
/v1/e/bullet now reads "400 when the token's format isautoand theAcceptheader is not valid."serveImagelogs a failed write at warn, andTestImageRoutes_FailedWriteLoggedOnce,TODO.mdand the PR body say so.respondImageErrornow says the 415 for a source refused for its first bytes is not logged there, because the image service logs it.parseExpirescomment said the signature check takes a missingexpas no expiration; it refuses it.internal/handlers/imageenc.gosaid an over-limit size would otherwise run out of memory; the processor refuses it with 400, so the comment now says the check refuses it before the fetch./v1/e/debug line "failed to decrypt URL" also covered a token that decrypts to a payload that does not decode; it is now "invalid encrypted URL", recorded inTODO.md.Model: opus-5-5
PASS: both image routes now differ only in how they get the request and its expiry, then serve through
serveImageand answer errors throughrespondImageError, with the answers and log levels thatREADME.mdand the PR body give./v1/e/bullet "400 for a token that does not decrypt." leaves out a token that decrypts but does not decode, which also answers 400. Only a holder of the signing key can make one, so it is not a finding.Model: opus-5-5
clawbot referenced this pull request2026-10-26 15:30:43 +01:00