feat(exceptions): expose the server's error code on NotFoundException - #326
mjbradford89 wants to merge 1 commit into
Conversation
Every 404 now carries the envelope's errors['code'] as NotFoundException.code (None when the server sent none — an endpoint 404, a non-JSON body, an older server). Callers could previously only tell "bytes withheld by design" or "bytes were never stored" from a plain miss by matching on the message prose, which the server may reword; the codes are wire-frozen. KnownGoodWithheldException keeps its own class — consumers already catch it — and sets .code to KNOWN_GOOD so the attribute reads uniformly whether or not the subclass is caught. New codes get .code values rather than new subclasses, which would break narrow `except NotFoundException` consumers.
ReviewMechanically the change is right: Three things need action. 1. The PR's own premise is only asserted against a fabricated body
That bites harder here than it did for Two concrete gaps, one free and one worth a re-record:
2.
|
SummaryWhen the platform refuses a download or rescan for a hash whose bytes it deliberately never stored, it said "the file does not exist" — the same words a genuinely absent artifact gets. The reporter read that as data loss and spent an investigation hunting a retention bug across 297 files in 47 container images. The server side splits the refusal on the row's persisted state so it names the cause, leaving Severity: 0 HIGH · 3 MODERATE · 5 LOW. Prior feedback: 9 checked · 9 open.
Fixes are proposed, not applied; nothing was run. Cross-repo coordination
Merge order: Contracts crossing the set: Coherence: fixes are independent; no cross-repo adjustment needed. Gaps: Findings (round 1)Every finding below is work for this change set; each entry's
elsewhere: F1, F2, F4, F8 → polyswarm/artifact-index#1978 · F3, F7 → polyswarm/polyswarm-cli#273 Outstanding review feedback
Standards conformitySet-level. §14 delivery order ✓ — the server PR ships no capability needing SDK or CLI support, since Change-level. §15 ✗ → F5. Project-level. §16 ✓ — no version bump is owed: no sibling in this set adopts Clean: §2, §16. Non-clean: §15 ✗. Not applicable: §3–§14, §17 — a client library, so no queues, storage, AKM, charts, clusters or request paths. |
SummaryWhen the platform refuses a download or rescan for a hash whose bytes it deliberately never stored, it said "the file does not exist" — the same words a genuinely absent artifact gets, which sent the reporter hunting a retention bug across 297 files in 47 container images. The server now answers that row first, above Severity: 0 HIGH · 1 MODERATE · 6 LOW. Prior feedback: 13 checked · 10 open.
Fixes are proposed, not applied; nothing was run. Cross-repo coordination
Merge order: Contracts crossing the set: Coherence: fixes are independent; no cross-repo adjustment needed. Gaps: Findings (round 2)Every finding below is work for this change set; each entry's
elsewhere: F1, F6 → polyswarm/polyswarm-cli#273 · F2, F3, F7 → polyswarm/artifact-index#1978 Outstanding review feedback
Standards conformityAudited in round 1 for all three members; that block stands. No new change-level violation in this PR — it is byte-identical to round 1. Set-level rows unchanged: §14 ✓, Rule 6 ◐ ( |
2 participants
TL;DR
errors['code']asNotFoundException.code(Nonewhen the server sent none).KnownGoodWithheldExceptionkeeps its own class and sets.codetoKNOWN_GOOD— no behaviour change for anyone catching it.Why
A 404 from the platform API carries a machine-readable
errors.code, and today the SDK reads exactly one value of it:KNOWN_GOOD, which routes to theKnownGoodWithheldExceptionsubclass. Every other code —NOT_STORED,DELETED,EXPIRED— was dropped on the floor, so a caller wanting to distinguish them had onlystr(exc).That is a bad thing to depend on. The codes are wire-frozen members of the server's state enum; the human message is prose the server may reword — and it just did, for
NOT_STORED, which previously read "the file does not exist" for bytes the platform deliberately never stored. A downstream consumer that had string-matched on that would have broken silently.NOT_STOREDis the case that motivated this: it means the platform knows the hash and intentionally never kept its bytes (it was declined as known-good at submission). Resubmitting the file works. That is materially different from "not found", and callers that cache verdicts need to tell them apart.Why an attribute rather than a subclass
Adding a subclass per code would break invariant 3 of the downstream contract for anyone catching
NotFoundExceptionnarrowly.KnownGoodWithheldExceptionexists because it predates this and consumers already catch it; it now sets.codetoo, so the attribute reads uniformly whether or not the subclass is caught. New codes get.codevalues, not new classes — recorded inspecs/05-downstream-contract.md.Test plan
test_404_other_error_code_stays_plain_not_foundnow asserts.codeacrossDELETED,NOT_STORED,EXPIRED, a legacy list-shapederrors, and noerrorsat all; the known-good test pins.code == 'KNOWN_GOOD'on the subclass.Full suite: 231 passed.
Companion PRs
Same branch name, independently mergeable — no ordering dependency:
polyswarm-cli— renders a distinct status forNOT_STOREDinstead of falling through to "Assertion window closed". It does not consume.code; it reads the instancestatethat already ships.NOT_STOREDrefusal message itself.errors.codeis unchanged there, which is precisely why this attribute is the stable thing to branch on.