feat(backend): log the reason on every error + more meaningful messages #43
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/verbose-error-logging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Error responses now explain themselves in the server log and to the client,
instead of leaving a bare status code.
serving paths are skipped so large downloads are never buffered) and, for 4xx,
logs the RFC 7807
detailalongside the status. 5xx already log detail+stackvia GlobalExceptionMiddleware.
"Conflict") instead of a generic "Error", across all call sites.
and the batch-size guidance; variant-not-found names the id.
rollback/cleanup preserved via a dedicated UploadRejectedException.
Tests: middleware now logs the 4xx reason; ZIP-too-many returns 400; existing
variant-not-found assertions relaxed to substring. Full backend suite green.
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 85%
DoujinManager.Infrastructure - 93.6%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
DoujinManager.RestAdapter - 84%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 26%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ error handling that actually explains itself? You're speaking my language, Björn ♡ Nothing delights me more than a request that whispers its reason into the log instead of leaving me a bare, mysterious little
400to divine. I read every byte of these 245 additions and, fufu~, they're tidy.The
UploadRejectedExceptionsplit is genuinely lovely — a client mistake becoming an honest 400 instead of masquerading as a 500, with the rollback + cleanup carefully preserved and pulled intoCleanupOrphanImagesAsync. Thecatch (UploadRejectedException)sitting before the genericcatch (Exception)is exactly the right order (specific first, greedy last~).TitleForStatusis a clean RFC 7807 phrase table. And the middleware body-capture correctly excludes the binary-serving paths so we never buffer a giant download just to log it. Chef's kiss.Verdict: ✅ Looks good to me~
No blockers. Security scan clean (no secrets, no injection, no
eval/exec— I looked, I always look ♡). CI is green for this exact head, so I didn't duplicate the work.💡 Little ideas (non-blocking)~
RequestLoggingMiddleware.cs:34— thatbuffer = new MemoryStream()never gets disposed~ It's harmless (GC sweeps it up), but ausing/await usingor atry/finallydispose after the copy-back would be tidier. Silly little untidiness♪RequestLoggingMiddleware.cs:118(ExtractErrorDetail) — you truncate the body atMaxCaptureBytes(16 KB) then feed it toJsonDocument.Parse. For your tiny problem+json bodies this never bites, but a >16 KB JSON body would fail to parse and silently fall to the raw-text branch. Since problem bodies are always small it's fine — just be aware the "parse a possibly-truncated string" combo is a touch fragile. Also, cutting at an arbitrary byte boundary can split a multi-byte UTF-8 char into a � — cosmetic, log-only.RequestLoggingMiddleware.cs:38— swappingcontext.Response.Bodyfor aMemoryStreamand copying back infinallyworks perfectly for the buffered JSON that minimal-APIResults.Problemwrites~ But it bypassesIHttpResponseBodyFeature, so if a capture-eligible endpoint ever writes straight toResponse.BodyWriter(the pipe) or callsStartAsync()itself, this capture would miss it and could interfere. Not today's problem — just worth a one-line comment near the swap so future-you doesn't accidentally add a streaming JSON endpoint under a captured path. Fufu~ I'd hate for a silly little bug to sneak in later, wouldn't you? ♡ExtractErrorDetail(non-JSON body) would close the last little gap.Everything substantive is correct and the tests prove the two behaviors that matter. Ship it whenever you like~ ♪
Automated review by Jibril · 2026-07-02
CI/CD: ✅ passed for head
fa87aed7(CI / build 21s, CI / test 51s) · Local checks: skipped per CI-evidence policy · Static scan: clean