Skip to content

Auth middleware: database failures are a 503 with a body, not an empty 500; fix the flaky parallel-install test - #117

Merged
fylorn merged 1 commit into
devfrom
fix/mcp-install-race
Oct 20, 2026
Merged

fylorn merged 1 commit into
devfrom
fix/mcp-install-race

Conversation

@fylorn

@fylorn fylorn commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Summary

mcp_store_concurrency::parallel_installs_same_template_keep_count_in_sync failed intermittently on dev with 500 Internal Server Error and an empty body.

Root cause

Not the install transaction (advisory lock, FOR UPDATE on the template row and install_count = install_count + 1 are all correct, and Postgres logged no error at the time of either failure). The 500 came from the console auth middleware:

  1. The test harness ground each login's proof of work (difficulty 19, about 500k SHA-256 tries on average, with a long geometric tail) inline on the #[tokio::test] current-thread runtime that also drives the in-process server.
  2. The test logged in session k+1 while install k was already in flight, so a slow grind froze that install mid-request.
  3. Its connection-pool acquire then outlived sqlx's 5 s acquire_timeout, even though the pool held idle connections. With temporary logging, every reproduced failure showed pool timed out while waiting for an open connection with pool_size=4..5, pool_idle=2..3 out of 20.
  4. require_auth mapped that error to a bare StatusCode::INTERNAL_SERVER_ERROR. The permission lookup did this without any log line, which is why CI showed only request failed ... 500 latency_ms=5004.

Changes

  • Product (auth_guard.rs, api_key_auth.rs): database errors in the session check, the console API-key path and the gateway API-key path now use the same From<sqlx::Error> for AppError mapping as handlers. A pool timeout is a 503 with the standard JSON error body; any other database error is a 500 with one. Each is logged with the name of the lookup that failed. Unit tests cover both mappings.
  • Harness (test-support/src/client.rs): the proof of work is ground on spawn_blocking, so the server keeps serving while a test client logs in. This affects every test that logs in while requests are in flight.
  • Test (mcp_store_concurrency.rs): all sessions log in before any install fires, so the five installs actually race each other instead of trickling out one login apart.
  • CHANGELOG [Unreleased] Fixed entry.

Reproduction (local, Postgres 18 + Redis 8 in dedicated containers, CPU saturated with busy loops to mimic a loaded runner)

Build Runs Failures
dev (before) 25 4 (three empty 500s from require_auth, one 503 from the handler's own pool acquire)
harness fix only, original test order 25 0
full fix 30 0

With the machine idle, both before and after pass 10/10. Locally: clippy -D warnings (lib/bins and tests), fmt and nextest --lib --bins (840 passed) all pass. The integration suite run without ClickHouse gave 386 passed; all 72 failures were ClickHouse-database creation.

🤖 Generated with Claude Code

…t an empty 500

The parallel-install integration test failed now and then with an empty
500. The cause was not the install transaction: the test harness ground
each login's proof of work inline on the current-thread runtime that
also drives the in-process server. A slow grind froze an install already
in flight, its connection-pool acquire outlived the 5 s acquire timeout
(with idle connections in the pool), and the session check in
require_auth mapped the error to a bare 500 without logging it.

- auth_guard / api_key_auth: database errors in the session and API-key
  checks go through the same mapping as handlers (pool timeout -> 503
  with a JSON body, other errors -> 500 with one) and are logged with
  the lookup that failed.
- test-support: grind the proof of work on the blocking pool so the
  server keeps running while a client logs in.
- mcp_store_concurrency: log every session in before firing the
  installs, so they actually race each other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fylorn
fylorn merged commit caa6751 into dev Oct 20, 2026
7 checks passed
@fylorn
fylorn deleted the fix/mcp-install-race branch October 20, 2026 14:50
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant