Skip to content

Database outage renders as 500 internal_error, not the retryable 503, on /ipfs/pins, /ipfs/{cid} and /arweave/anchors #251

Description

@beardthelion

What happens

On main (111cff7), when the database is unreachable, three routes return 500 internal_error where every sibling route returns 503 db_unavailable. Observed through the real mounted router against a closed pool, all four requests anonymous and unsigned:

/api/v1/arweave/anchors -> 500 {"error":"internal_error","message":"attempted to acquire a connection on a closed pool"}
/api/v1/ipfs/pins       -> 500 {"error":"internal_error","message":"attempted to acquire a connection on a closed pool"}
/api/v1/peers           -> 503 {"error":"db_unavailable","message":"database is temporarily unavailable"}
/api/v1/repos           -> 503 {"error":"db_unavailable","message":"database is temporarily unavailable"}

Same process, same pool, same request cycle. The split is per handler, not a deliberate policy.

Why

The db layer returns anyhow::Result, and impl From<anyhow::Error> for AppError in crates/gitlawb-node/src/error.rs is the only place that downcasts a sqlx::Error back out of the anyhow chain into AppError::Db. IntoResponse then sends AppError::Db(e) through db_unavailable(e), which maps the connection-level variants (PoolTimedOut, PoolClosed, Io, Tls) to 503.

A handler that writes .map_err(AppError::Internal) rather than a bare ? constructs the Internal variant directly, so that downcast never runs and the error takes the unconditional 500 arm.

Sites on main:

  • crates/gitlawb-node/src/api/ipfs.rs:84
  • crates/gitlawb-node/src/api/ipfs.rs:94
  • crates/gitlawb-node/src/api/ipfs.rs:224
  • crates/gitlawb-node/src/api/arweave.rs:33

Why it matters

503 is the code that means "temporary, retry", and 500 means "this request is broken". A proxy, an uptime check, or a client retry policy keyed on 503 will not fire for these routes during a database outage, so a recoverable condition reads as a hard failure on exactly the metadata endpoints most likely to be polled. All three routes are unauthenticated (/api/v1/ipfs/pins and /api/v1/arweave/anchors carry no auth layer, /ipfs/{cid} carries only optional_signature), so the behavior is observable by anyone.

Scope

Pre-existing, and not introduced by #247. That PR makes the body of both 500 arms opaque, which is a separate and correct change; the status split survives it, which is why this is its own issue rather than a review comment there.

#134 is currently in flight on api/ipfs.rs and api/arweave.rs. It rewrites the arweave call site and adds three more .map_err(AppError::Internal)? sites, so a fix here should land after it or be coordinated with it, and should cover the sites #134 adds rather than only the four above.

Fix direction

Use a bare ? at these call sites so the existing From<anyhow::Error> conversion runs and connection-class failures reach the 503 arm. Non-sqlx errors still land in Internal through that same conversion, so nothing else changes shape. I have not compiled that change, so this is the direction rather than a verified patch.

A regression test would drive one of these routes against a closed pool and assert 503 plus db_unavailable, which is the shape the reproduction above already uses: build the router from a migrated test pool, call pool.close(), then issue the request.

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

    crate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:lowCosmetic, cleanup, or nice-to-havesubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions