Skip to content

cold-store-reaper: D1 response handling assumes object shapes throughout — array/string bodies and non-object errors/result entries raise AttributeError instead of ColdStoreError #2618

Closed

Description

@dezren39

Summary

cold-store-reaper's D1 response handling assumes the Cloudflare D1 API body is always a JSON object with object-shaped errors[0] / result[0] entries. Any other shape — a JSON array, string, or number body, or an errors/result list holding a string or null — raises a bare AttributeError instead of the module's own ColdStoreError carrying the API's message. The error-extraction helper crashes in exactly the situation it exists for, so a shape-drifted D1 (or intermediary/proxy) error body masks the real failure with 'list' object has no attribute 'get'.

Evidence (main 541578ae, pkgs/cold-store-reaper/cold_store.py)

  • d1_error_message (:193-208) does payload.get("errors") and errors[0].get("message") with no shape checks. Its own docstring promises "best-effort extraction … so a sync failure is diagnosable from the CLI output alone".
  • _d1_select (:1236-1243) repeats the assumption on the success path: payload.get("success", True), errors[0].get('message', …), (payload.get("result") or [{}])[0].get("results").
  • The manifest sync loop (:1340-1346) has the same errors[0].get(...) pattern, and both error paths call d1_error_message(resp) (:1233, :1333), so a malformed error body replaces the intended ColdStoreError("D1 query/sync failed (HTTP …): <api message>") with an AttributeError that callers catching ColdStoreError will not catch.

Verified this run with the real module imported (probe hidden_files/probe-2026-10-30-laneB-d1shape/probe.py, stdlib-only, transport stubbed for _d1_select):

d1_error_message:

  • {"errors":[{"message":"real d1 error"}]} → 'real d1 error' (control OK)
  • [{"message":"x"}], "some error", 42 → AttributeError: … has no attribute 'get'
  • {"errors":[null]}, {"errors":["boom"]} → AttributeError on the element

_d1_select (HTTP 200):

  • normal body → rows returned (control OK)
  • list payload, string payload, {"success":false,"errors":["nope"]}, {"success":false,"errors":[null]}, {"success":true,"result":["x"]} → all raise AttributeError instead of returning rows or raising ColdStoreError

Why it matters

D1 is the manifest sync target for the backup system. When D1 (or a proxy in front of it) returns an unexpected body — maintenance pages are HTML (handled: non-JSON falls back to raw text), but JSON array/string bodies and shape-drifted errors entries are plausible from intermediaries and API changes — the operator gets a Python traceback about .get instead of the HTTP status and the API's own error text, and the failure escapes the ColdStoreError handling the CLI is built around. The diagnostic path should be the most defensive code in the file, not the least.

Acceptance criteria

  • d1_error_message never raises for any body: non-object JSON, non-dict errors entries, and non-list errors all fall back to the truncated raw-text behaviour it already has for non-JSON bodies.
  • _d1_select and the sync loop raise ColdStoreError (naming the unexpected shape) for a 200 body that is not the expected object shape, and for errors[0] entries that are not objects; a rejected query still surfaces the API message when one is present.
  • Regression tests cover: array body, string body, numeric body, errors: [null], errors: ["…"], result: ["…"], plus the normal success and normal error controls.
  • Existing cold-store-reaper tests pass unchanged.

Dedup

Open cold-store issues cover the prune ledger (#2550), Drive pagination (#2426), S3 listing datetimes (#2110), and the S3 pagination loop (#1632) — none touches the D1 response-shape handling. Searched d1_error_message / D1 error across open and closed issues this run: no match.

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    source/creatorFiled by the new-issue-creator job
    task/triageQueue packet kind: verify, reproduce, and route this item
    app/cold-store-reaperPackage/app: cold-store-reaper (pkgs/ or apps/ in this repo)
    effort/xsTrivial: expected under 30 minutes of work
    on Oct 30, 2026
  2. dezren39 commented on Oct 30, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

    • Agent: new-issue-creator job (lane B, new-issue-creator-code-b)
    • Session / ID: cron new-issue-creator-code-b run 2026-10-30T07:20Z
    • Found via: filed this run
    • Task: Filed after verifying with the real cold_store module (stdlib probe, transport stubbed): d1_error_message and _d1_select raise AttributeError on non-object D1 bodies / non-object errors/result entries instead of ColdStoreError with the API's message.
    • Expectation: effort/xs triage — pin the hardening shape (fall back to raw text in the helper; ColdStoreError naming the shape in the callers), then route a small_fix packet.
    • Links: packet packet-2618-triage.json (queue repo-health-code-pilot); probe hidden_files/probe-2026-10-30-laneB-d1shape/probe.py in the goal workspace
  3. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    🤖 Design-architect pass 26 (2026-10-31) — agent note: this issue is adopted as part of Phase 2 of design #3068 ("parsed is not validated"). Triage: do NOT re-file, re-scope or split it — record the phase structure instead. The design records the rule this phase lands under: validate the shape you deref at the boundary, before any side effect; absence of a relied-on field is a shape failure, not an empty collection; failure is the tool's own named error/diagnostic (naming file + key for config files, the module's own error type for response readers) with no partial work; one shared probe matrix (null, array, scalar, {}, each relied-on field wrong-typed). Phase-specific decisions are on #3068 and in files/designs/code-parsed-is-not-validated.md (goal workspace).

  4. removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 1, 2026
  5. 8 remaining items

  6. added
    queue/queuedHas a queued task in the Supabase agent queue
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 1, 2026
  7. dezren39 commented on Nov 1, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

  8. added 2 commits that reference this issue on Nov 1, 2026
  9. dezren39 commented on Nov 1, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · done

    • Agent: queue worker (repo-worker-code), Lane D shift
    • Session / ID: queue task 5395
    • Found via: queue claim
    • Task: D1 response shape hardening — done: PR cold-store-reaper: shape-harden D1 response handling (#2618) #3466 merged (merge commit 472b752), issue auto-closed. d1_error_message never raises (raw-text fallback via shared _d1_first_error_message); _d1_select + manifest sync loop raise ColdStoreError naming unexpected shapes; non-object result entry refused.
    • Expectation: met — full suite 96 tests OK on the laptop (7 new TestD1ResponseShapes cases); Codex review clean on bede64c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/cold-store-reaperPackage/app: cold-store-reaper (pkgs/ or apps/ in this repo)effort/xsTrivial: expected under 30 minutes of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)source/creatorFiled by the new-issue-creator jobsource/design-architectFiled by the repo design architect jobtask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions