fix(db): give every query an abort budget - #33
Conversation
`createDatabase` built the Neon client with no fetch options, so every query in the system inherited undici's defaults. That is the one uninstrumented transport here: heartbeat, supervisor nudge and chat TTFB all carry an explicit budget already. It matters in the mode this system exists to survive. A refused connection fails fast either way, but a store that accepts TCP and never answers made every caller wait on the transport, and the routing pointer is read both on the chat hot path (`cachedSnapshot`) and inside `resolveStepTimeout`, where the reconciler decides whether a timed-out promotion actually committed. A hang there leaves the operation neither done nor failed. The signal is per call and rides Neon's `fetchOptions`, so an expired budget genuinely aborts the fetch. Racing a timer against the promise would stop the waiting without cancelling the request, accumulating one abandoned socket per attempt during exactly the outage this is meant to contain. The budget is wrapped around `client.query`, not the callable form: drizzle's neon-http session resolves `client.query ?? client` once at construction and the Neon client does expose `query`, so budgeting the callable form would compile, pass its own tests, and never apply to a real query. An expired budget surfaces as a rejection, which the fail-open contract requires rather than merely tolerates: `cachedSnapshot` reads a throw from the pointer read as "the store is unreachable", and that throw is the only thing licensing a stale route. No new env knob, per the KNOWN_ISSUES entry's stated reason for deferral. That entry is narrowed rather than deleted: the transport half is bounded now, the memoized "store is down" half is still open.
WalkthroughThe database client now applies configurable per-query timeouts through abort signals. Drizzle receives the wrapped Neon client. Tests cover timeout behavior, lazy queries, transactions, and fetch integration. Known-issues documentation describes remaining outage memoization work. ChangesQuery budget enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant createDatabase
participant withQueryBudget
participant Drizzle
participant NeonClient
createDatabase->>withQueryBudget: wrap Neon client with configured budget
withQueryBudget->>Drizzle: provide wrapped client
Drizzle->>NeonClient: execute query with per-request AbortSignal
NeonClient-->>Drizzle: return result or timeout rejection
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review BotNo comment/code divergences or documentation drift detected. Reviewed 6 file(s); skipped 0. |
Greptile SummaryThe PR adds a configurable per-query abort budget to the Neon database transport while preserving caller cancellation, lazy query behavior, and transaction batching.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/db/src/client.ts | Adds validated per-call timeout composition around the Neon query and transaction paths while retaining lazy query objects and clearing timers after settlement. |
| packages/db/src/client.test.ts | Exercises the actual Drizzle query path, option and signal composition, transaction batching, timer cleanup, invalid configuration, and driver response edge cases. |
| packages/protocol/src/http.ts | Exports the existing timeout-signal helper and makes its signal inputs optional for database transport reuse. |
| packages/protocol/src/policy.ts | Exports the shared timeout validation schema so database budgets follow the repository's established timer bounds. |
| KNOWN_ISSUES.md | Narrows the documented chat outage issue to repeated per-request latency now that transport waits are bounded. |
| KNOWN_ISSUES.ja.md | Applies the corresponding known-issue update to the Japanese documentation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Repository operation] --> B[Drizzle Neon session]
B --> C[Budgeted client query or transaction]
C --> D[Compose caller signal with query budget]
D --> E[Neon HTTP request]
E -->|Settles| F[Clear budget timer]
E -->|Budget expires| G[Abort request and reject]
Reviews (10): Last reviewed commit: "docs(db): stop the budget comment claimi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/client.ts`:
- Around line 78-81: Update withQueryBudget to validate budgetMs before creating
the wrapped client, rejecting NaN, Infinity, negative, and non-integer values
before AbortSignal.timeout is reached; explicitly preserve the intended
zero-budget behavior as either a permitted pass-through or rejection. Add
coverage for fractional invalid values and ensure valid budgets continue to wrap
the client normally.
- Around line 95-102: Update the fetchOptions construction in the target.query
call to preserve callerFetchOptions.signal: when a caller signal exists, combine
it with the budget timeout using AbortSignal.any, while retaining the
timeout-only behavior when no caller signal is provided. Add a test covering
caller cancellation before budget expiration and verify the fetch aborts
promptly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8186c28e-ab6b-4c65-a454-feff69b02c81
📒 Files selected for processing (4)
KNOWN_ISSUES.ja.mdKNOWN_ISSUES.mdpackages/db/src/client.test.tspackages/db/src/client.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Seer Code Review
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (12)
{README,CONTRIBUTING,KNOWN_ISSUES}{,.ja}.md
📄 CodeRabbit inference engine (AGENTS.md)
Maintain English/Japanese documentation pairs together: editing one side requires updating the other in the same change.
Files:
KNOWN_ISSUES.mdKNOWN_ISSUES.ja.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Do not reference consumer-private repositories or infrastructure, specific model names, or specific GPU names in code, comments, tests, docs, seeds, or example layouts.
**/*: Use kebab-case for file names.
Write comments in English.
Do not use the em dash character (U+2014) in code or prose; use a colon, comma, parentheses, or spaced hyphen instead.ファイル名は kebab-case にする。
Files:
KNOWN_ISSUES.mdpackages/db/src/client.tsKNOWN_ISSUES.ja.mdpackages/db/src/client.test.ts
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use English code comments and prose, and avoid the em dash character U+2014.
Files:
KNOWN_ISSUES.mdpackages/db/src/client.tsKNOWN_ISSUES.ja.mdpackages/db/src/client.test.ts
**/*.{ts,tsx,js,jsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use oxfmt as the owner of formatting, including whitespace, wrapping, quotes, and trailing commas; do not hand-tune formatting for ESLint.
Files:
KNOWN_ISSUES.mdpackages/db/src/client.tsKNOWN_ISSUES.ja.mdpackages/db/src/client.test.ts
**/*.{ts,tsx,js,jsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
oxfmt を使用して、空白、折り返し、クォート、末尾カンマを整形する。整形確認には
pnpm format:checkを使用する。
Files:
KNOWN_ISSUES.mdpackages/db/src/client.tsKNOWN_ISSUES.ja.mdpackages/db/src/client.test.ts
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
コードと文章ではエムダッシュ (U+2014) を使用せず、コロン、コンマ、括弧、またはスペース付きハイフンを使用する。
Files:
KNOWN_ISSUES.mdpackages/db/src/client.tsKNOWN_ISSUES.ja.mdpackages/db/src/client.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Respect the dependency graph:@haru/protocolcontains shared types/helpers;@haru/coreis pure logic with no I/O;@haru/dbdepends on core; drivers, server, and supervisor must only use permitted dependencies. Shared server/supervisor code belongs in@haru/protocol.
Build outbound URLs withjoinUrlfrom@haru/protocol; do not usenew URL('/path', base)because it drops a base path prefix.
Keep all I/O injectable, including fetch, exec, spawn, and clocks.
Use the repository’s root linter and formatter configurations; do not add per-package linter configurations, and do not manually align formatting. Add root overrides with a reason comment when needed.Keep new I/O behind injectable boundaries so it can be tested without GPUs, cloud accounts, or a running database.
新しい I/O は注入可能な境界の背後に配置し、外部実行、fetch、子プロセス、タイマーなどをテストダブルに置き換えられるようにする。
Files:
packages/db/src/client.tspackages/db/src/client.test.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/db/src/**/*.{ts,tsx}: Implement every state transition as a single-statement compare-and-swap using an appropriateUPDATE ... WHERE state IN (...) RETURNINGand verify the affected row count. Never usedb.transaction()or hold external work between a read and its dependent write.
Enforce the core state tables as the single source of truth: repository code must reject invalid(from, to)pairs withInvalidTransitionError.
Files:
packages/db/src/client.tspackages/db/src/client.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run both root-configured linters,
oxlint --type-awarefollowed by strict type-aware ESLint 10; add overrides at the repository root rather than per-package configs.
**/*.{ts,tsx,js,jsx}: oxlint の型認識 lint を実行した後、型情報ベースの strict ESLint 10 を実行する。設定はパッケージごとではなくリポジトリルートに置き、例外には理由をコメントしたスコープ付きオーバーライドを優先する。
コード内のコメントは英語で記述する。
Files:
packages/db/src/client.tspackages/db/src/client.test.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test
@haru/dbagainst in-memory PGlite using the committed Drizzle migrations, including compare-and-swap SQL and concurrent-winner races.状態ストアの状態遷移では、compare-and-swap SQL によって並行実行時の勝者決定レースを保護する。
Files:
packages/db/src/client.tspackages/db/src/client.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Use@haru/db/testingand its committed migrations for database tests; do not add per-test migration calls. UseloadExampleFleetLayoutfor shared example layouts.
Server tests should drive Hono withapp.request()and scripted fake supervisors; supervisor tests should use fake timers for SIGTERM-to-grace-to-SIGKILL escalation.
Files:
packages/db/src/client.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add a Vitest case next to changed code when introducing or modifying behavior.
変更したコードの近くに Vitest のテストを追加し、外部 I/O は注入可能な境界に対してテストする。
Files:
packages/db/src/client.test.ts
🪛 LanguageTool
KNOWN_ISSUES.md
[typographical] ~48-~48: The word ‘Why’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...slice deliberately adds no new env knobs. - Intended fix: a short-lived "store is...
(WRB_QUESTION_MARK)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed8d985985
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…path
Review feedback, all three verified locally first.
Sentry was right that the caller's signal was overwritten rather than
composed. `packages/protocol/src/http.ts` already solves exactly this and
says so in a comment ('a signal the caller put in init must compose, not
be silently discarded'), so `timeoutSignal` is exported and reused
instead of a second implementation. That also replaces
`AbortSignal.timeout`, whose timer cannot be cleared: the budget timer is
now released in a `finally` when the query settles, so a settled query
no longer holds one for the full budget.
Greptile read the wrapper as leaving the callable path unbounded, so
normal selects would bypass the signal. That is not what the driver does,
but the honest answer to 'which path does drizzle take' is a test rather
than a claim, so there is now one that drives real drizzle against a
recording fake client and asserts the signal reached the issued query. It
is the test that would have caught budgeting the wrong path, which was a
real hazard here and not a hypothetical one.
|
Thanks - worked through all three, verified each locally before changing anything. Pushed as @sentry (composing the caller's signal): correct, fixed. I was overwriting That swap fixed something I had not flagged: @greptile-apps (P1, callable path unbounded): not what the driver does — but the objection deserved a test, not a rebuttal. The honest answer to "which path does drizzle actually take" is a test, so there is one now: it builds a real @coderabbitai (budget value validation): the budget is not user input on any path today (a constant, or an explicit argument from a caller inside this repo), so I left it unguarded rather than adding a check with no caller. Happy to add one if you would rather have the constructor reject a non-finite or non-positive value. Also added a case pinning the composition (aborting the caller's controller aborts the query's signal) and one asserting the timer count does not grow across repeated queries, measured as a delta since the runner keeps timers of its own.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
|
Good work on all three — the implementation is now correct and well-evidenced. Callable path: You were right, I was wrong. Signal composition via Timer cleanup: On budget validation: your reasoning holds. Both sources of This is in good shape to merge. Tip: You can customize Greptile's behavior for this repo with |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/client.test.ts (1)
209-215: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the exact default query budget.
The contract specifies a 10-second default. A range assertion also accepts 30 seconds and does not protect that contract.
Assert that
DEFAULT_QUERY_BUDGET_MSequals10_000.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/client.test.ts` around lines 209 - 215, Update the test for DEFAULT_QUERY_BUDGET_MS to assert that it equals the contract’s exact 10-second value, 10_000, instead of only checking a positive bounded range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/client.test.ts`:
- Around line 195-202: Update the Drizzle query assertion in the integration
test so it no longer catches and ignores every error: make the fake client
return a valid Neon fullResults response, or catch only the expected
response-shape mismatch and assert its exact details. Ensure unexpected wrapper
or integration failures propagate and fail the test.
In `@packages/db/src/client.ts`:
- Around line 100-104: Update withQueryBudget to accept an injectable
timeout-signal factory, defaulting to timeoutSignal, and use that factory where
the query budget signal is created. Preserve the existing budgetMs,
callerFetchOptions.signal, and returned signal/timer behavior while allowing
tests to provide a fake clock.
---
Outside diff comments:
In `@packages/db/src/client.test.ts`:
- Around line 209-215: Update the test for DEFAULT_QUERY_BUDGET_MS to assert
that it equals the contract’s exact 10-second value, 10_000, instead of only
checking a positive bounded range.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0c843926-2df4-40c7-a7f7-998d62b69d56
📒 Files selected for processing (3)
packages/db/src/client.test.tspackages/db/src/client.tspackages/protocol/src/http.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Seer Code Review
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Respect the dependency graph:@haru/protocolcontains shared types/helpers;@haru/coreis pure logic with no I/O;@haru/dbdepends on core; drivers, server, and supervisor must only use permitted dependencies. Shared server/supervisor code belongs in@haru/protocol.
Build outbound URLs withjoinUrlfrom@haru/protocol; do not usenew URL('/path', base)because it drops a base path prefix.
Keep all I/O injectable, including fetch, exec, spawn, and clocks.
Use the repository’s root linter and formatter configurations; do not add per-package linter configurations, and do not manually align formatting. Add root overrides with a reason comment when needed.Keep new I/O behind injectable boundaries so it can be tested without GPUs, cloud accounts, or a running database.
新しい I/O は注入可能な境界の背後に配置し、外部実行、fetch、子プロセス、タイマーなどをテストダブルに置き換えられるようにする。
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Do not reference consumer-private repositories or infrastructure, specific model names, or specific GPU names in code, comments, tests, docs, seeds, or example layouts.
**/*: Use kebab-case for file names.
Write comments in English.
Do not use the em dash character (U+2014) in code or prose; use a colon, comma, parentheses, or spaced hyphen instead.ファイル名は kebab-case にする。
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use English code comments and prose, and avoid the em dash character U+2014.
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use oxfmt as the owner of formatting, including whitespace, wrapping, quotes, and trailing commas; do not hand-tune formatting for ESLint.
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run both root-configured linters,
oxlint --type-awarefollowed by strict type-aware ESLint 10; add overrides at the repository root rather than per-package configs.
**/*.{ts,tsx,js,jsx}: oxlint の型認識 lint を実行した後、型情報ベースの strict ESLint 10 を実行する。設定はパッケージごとではなくリポジトリルートに置き、例外には理由をコメントしたスコープ付きオーバーライドを優先する。
コード内のコメントは英語で記述する。
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
oxfmt を使用して、空白、折り返し、クォート、末尾カンマを整形する。整形確認には
pnpm format:checkを使用する。
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
コードと文章ではエムダッシュ (U+2014) を使用せず、コロン、コンマ、括弧、またはスペース付きハイフンを使用する。
Files:
packages/protocol/src/http.tspackages/db/src/client.test.tspackages/db/src/client.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/db/src/**/*.{ts,tsx}: Implement every state transition as a single-statement compare-and-swap using an appropriateUPDATE ... WHERE state IN (...) RETURNINGand verify the affected row count. Never usedb.transaction()or hold external work between a read and its dependent write.
Enforce the core state tables as the single source of truth: repository code must reject invalid(from, to)pairs withInvalidTransitionError.
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Use@haru/db/testingand its committed migrations for database tests; do not add per-test migration calls. UseloadExampleFleetLayoutfor shared example layouts.
Server tests should drive Hono withapp.request()and scripted fake supervisors; supervisor tests should use fake timers for SIGTERM-to-grace-to-SIGKILL escalation.
Files:
packages/db/src/client.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add a Vitest case next to changed code when introducing or modifying behavior.
変更したコードの近くに Vitest のテストを追加し、外部 I/O は注入可能な境界に対してテストする。
Files:
packages/db/src/client.test.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test
@haru/dbagainst in-memory PGlite using the committed Drizzle migrations, including compare-and-swap SQL and concurrent-winner races.状態ストアの状態遷移では、compare-and-swap SQL によって並行実行時の勝者決定レースを保護する。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
🔇 Additional comments (2)
packages/db/src/client.ts (1)
79-82: ValidatebudgetMsbefore creating the proxy.The previous finding remains.
withQueryBudgetaccepts invalid budgets and defers their normalization to the timer API. Require a finite non-negative integer. Define whether zero disables the budget or aborts immediately.packages/protocol/src/http.ts (1)
7-18: LGTM!
Review raised a P1 that the signal never reaches fetch, because `fetchOptions` would be construction-time only and the fake was inventing a parameter the driver ignores. Checked it against the installed @neondatabase/serverless 1.1.0 rather than arguing: `HTTPQueryOptions` declares `fetchOptions`, the published examples pass it directly to `query()`, and intercepting `neonConfig.fetchFunction` shows the signal arriving at the fetch layer as the same object. So the budget does apply. The objection was still pointing at a real gap. Every other case here asserts what the wrapper hands a FAKE, which only means something while the fake reproduces the driver's contract, and nothing was checking that. A fake inventing an ignored parameter is exactly how this would have become a silent no-op. So the probe that settled it is now a test: it drives the real driver with an intercepted fetch and asserts query-level `fetchOptions` reaches it. If a future release moves the option to construction time, this fails instead of the budget going quietly inert. `neonConfig` is process-global, so the stub is restored in a `finally`.
|
Checked the P1 against the installed driver rather than arguing it. Pushed as @chatgpt-codex-connector (P1, signal never reaches fetch): not what 1.1.0 does — verified three ways.
The signal arrives at the fetch layer as the same object. That said, the objection was aimed at a real gap, so I would rather close it than win it. Every other case in this file asserts what the wrapper hands a fake, which only means anything while the fake reproduces the driver's contract — and nothing was checking that. A fake that invents a parameter the driver ignores is precisely how this would have become a silent no-op, which is the failure mode this PR exists to prevent elsewhere. So the probe above is now a test: it drives the real driver with an intercepted fetch and asserts query-level @coderabbitai (inject the timeout-signal clock): leaving it as-is, and the reasoning may be worth disagreeing with. The wrapper deliberately delegates to The one place you have a point is the timer-leak assertion: reading
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18995ed738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The async wrapper was a real bug and the review caught it. Drizzle's batch path calls client.query() per statement and hands the resulting lazy NeonQueryPromise objects straight to client.transaction(), which reads queryData and opts off them. Awaiting inside the wrapper collapsed each one to a bare Promise, so batch would have lost the query data with nothing else failing. Confirmed against @neondatabase/serverless 1.1.0 rather than reasoning about it: a direct query() returns NeonQueryPromise with keys execute/queryData/opts, and the same call through an async wrapper returns a plain Promise with none of them. The wrapper is synchronous now and returns the driver's object untouched; the budget timer is released by wrapping that object's execute in place. transaction is budgeted too, since it is the single HTTP request the batch path actually sends, and budgeting only query would have left the one call that goes over the wire unbounded. Two regression tests. One asserts the returned object still carries queryData and matches the driver's own constructor, verified to fail (expected undefined to be defined) when the wrapper is reverted to async. The other asserts transaction receives a signal while keeping the caller's isolationLevel. Also stopped casting a caller-supplied signal blindly: a truthy non-AbortSignal in fetchOptions would have thrown inside AbortSignal.any, so it is instanceof-checked before composing.
|
Both of last night's findings were right. Fixed in @chatgpt-codex-connector (P2, lazy queries for batch): a real bug, thank you. Verified against Drizzle's The wrapper is synchronous now and returns the driver's object untouched; the timer is released by wrapping that object's Regression test asserts the returned object still carries @sentry (unvalidated signal cast): correct. Worth saying plainly: this PR argues that a wrapper can compile, pass its own tests and still not apply to a real query, and I then shipped a second instance of that in the same file. The lazy-object contract was invisible to every fake-based test here. Both of these were caught by review reading the driver rather than the diff, which is the part I had not done thoroughly enough.
|
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/db/src/client.ts (1)
153-156: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winInject the timeout-signal factory so the budget clock is testable.
withQueryBudgetreachestimeoutSignaldirectly throughbudgetedOptions, so every budget uses a real timer. Tests cannot substitute the clock. The suite compensates with wall-clock waits on a 20 ms budget, which is a known source of CI flakes on loaded runners. The coding guidelines require injectable clocks.Accept a factory parameter and default it to
timeoutSignal. Thread it throughbudgetedOptions.♻️ Proposed change
export function withQueryBudget<T extends NeonQueryClient>( client: T, budgetMs: number, + createTimeoutSignal: typeof timeoutSignal = timeoutSignal, ): T {function budgetedOptions( options: Record<string, unknown> | undefined, budgetMs: number, + createTimeoutSignal: typeof timeoutSignal = timeoutSignal, ): { merged: Record<string, unknown>; timer: ReturnType<typeof setTimeout> } { const callerFetchOptions = (options?.fetchOptions as Record<string, unknown> | undefined) ?? {}; const existing = callerFetchOptions.signal; - const { signal, timer } = timeoutSignal( + const { signal, timer } = createTimeoutSignal( budgetMs, undefined, existing instanceof AbortSignal ? existing : undefined, );As per coding guidelines, "Keep all I/O injectable, including fetch, exec, spawn, and clocks." <coding_guidelines>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/client.ts` around lines 153 - 156, Update withQueryBudget to accept an injectable timeout-signal factory parameter defaulting to timeoutSignal, then pass that factory through to budgetedOptions so callers and tests can control the budget clock without real timers.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/db/src/client.test.ts`:
- Around line 261-278: Extend the transaction wrapper tests around
withQueryBudget to verify timer cleanup on both resolved and rejected
transaction calls. Use the file’s existing timer-counting helper to capture the
initial pending-timer count and assert it is restored after each transaction
settles, while preserving the existing fetchOptions and caller-option
assertions.
- Around line 254-255: Remove the `.catch()` subscriptions for the `direct` and
`budgeted` lazy query objects in the test, leaving them unobserved so the test
continues to verify that no execution occurs. Preserve the existing assertions
and timer behavior without adding another rejection guard.
- Around line 238-251: The test should also verify that withQueryBudget applies
an abort signal to Neon’s lazy query options. In the budgeted query assertions,
read budgeted.opts?.fetchOptions?.signal and assert it is an AbortSignal, while
retaining the existing queryData assertions that validate the lazy
NeonQueryPromise shape.
In `@packages/db/src/client.ts`:
- Around line 68-79: Validate budgetMs at the budgetedOptions boundary before
calling timeoutSignal, rejecting non-finite, non-positive, or non-integer values
with an error that identifies the invalid configuration; explicitly treat 0 as
rejected rather than “no budget.”
- Around line 108-111: Guard result for null or undefined before accessing
execute in the lazy-result handling block, then preserve the existing
direct-result path and ensure the timer cleanup still runs when no executable
result is returned. Update the surrounding logic rather than relying on the
unchecked cast alone.
- Around line 101-121: Update clearTimerWhenSettled() and the withQueryBudget()
lazy-query handling so per-query timers do not keep the runtime open when
execute is never invoked. Immediately unref each timer where supported, and
ensure timers are cleared when their owning query promise is consumed or
abandoned, including multiple query objects passed to client.transaction().
---
Duplicate comments:
In `@packages/db/src/client.ts`:
- Around line 153-156: Update withQueryBudget to accept an injectable
timeout-signal factory parameter defaulting to timeoutSignal, then pass that
factory through to budgetedOptions so callers and tests can control the budget
clock without real timers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ba71e91e-4381-4877-b776-7da882fa99e0
📒 Files selected for processing (2)
packages/db/src/client.test.tspackages/db/src/client.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Seer Code Review
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Respect the dependency graph:@haru/protocolcontains shared types/helpers;@haru/coreis pure logic with no I/O;@haru/dbdepends on core; drivers, server, and supervisor must only use permitted dependencies. Shared server/supervisor code belongs in@haru/protocol.
Build outbound URLs withjoinUrlfrom@haru/protocol; do not usenew URL('/path', base)because it drops a base path prefix.
Keep all I/O injectable, including fetch, exec, spawn, and clocks.
Use the repository’s root linter and formatter configurations; do not add per-package linter configurations, and do not manually align formatting. Add root overrides with a reason comment when needed.Keep new I/O behind injectable boundaries so it can be tested without GPUs, cloud accounts, or a running database.
新しい I/O は注入可能な境界の背後に配置し、外部実行、fetch、子プロセス、タイマーなどをテストダブルに置き換えられるようにする。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/db/src/**/*.{ts,tsx}: Implement every state transition as a single-statement compare-and-swap using an appropriateUPDATE ... WHERE state IN (...) RETURNINGand verify the affected row count. Never usedb.transaction()or hold external work between a read and its dependent write.
Enforce the core state tables as the single source of truth: repository code must reject invalid(from, to)pairs withInvalidTransitionError.
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Use@haru/db/testingand its committed migrations for database tests; do not add per-test migration calls. UseloadExampleFleetLayoutfor shared example layouts.
Server tests should drive Hono withapp.request()and scripted fake supervisors; supervisor tests should use fake timers for SIGTERM-to-grace-to-SIGKILL escalation.
Files:
packages/db/src/client.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Do not reference consumer-private repositories or infrastructure, specific model names, or specific GPU names in code, comments, tests, docs, seeds, or example layouts.
**/*: Use kebab-case for file names.
Write comments in English.
Do not use the em dash character (U+2014) in code or prose; use a colon, comma, parentheses, or spaced hyphen instead.ファイル名は kebab-case にする。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use English code comments and prose, and avoid the em dash character U+2014.
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add a Vitest case next to changed code when introducing or modifying behavior.
変更したコードの近くに Vitest のテストを追加し、外部 I/O は注入可能な境界に対してテストする。
Files:
packages/db/src/client.test.ts
**/*.{ts,tsx,js,jsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use oxfmt as the owner of formatting, including whitespace, wrapping, quotes, and trailing commas; do not hand-tune formatting for ESLint.
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run both root-configured linters,
oxlint --type-awarefollowed by strict type-aware ESLint 10; add overrides at the repository root rather than per-package configs.
**/*.{ts,tsx,js,jsx}: oxlint の型認識 lint を実行した後、型情報ベースの strict ESLint 10 を実行する。設定はパッケージごとではなくリポジトリルートに置き、例外には理由をコメントしたスコープ付きオーバーライドを優先する。
コード内のコメントは英語で記述する。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test
@haru/dbagainst in-memory PGlite using the committed Drizzle migrations, including compare-and-swap SQL and concurrent-winner races.状態ストアの状態遷移では、compare-and-swap SQL によって並行実行時の勝者決定レースを保護する。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
oxfmt を使用して、空白、折り返し、クォート、末尾カンマを整形する。整形確認には
pnpm format:checkを使用する。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CONTRIBUTING.ja.md)
コードと文章ではエムダッシュ (U+2014) を使用せず、コロン、コンマ、括弧、またはスペース付きハイフンを使用する。
Files:
packages/db/src/client.test.tspackages/db/src/client.ts
🔇 Additional comments (1)
packages/db/src/client.ts (1)
52-62: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 976f09d37d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The previous commit fixed batch by keeping query() lazy, and in doing so introduced the opposite leak: a batched statement's timer was only cleared from the execute hook, and batch hands its lazy queries to transaction() without executing them individually. Measured before changing anything - building three lazy queries left three 60s timers outstanding, and nothing ever cleared them. Each build now tags its timer onto the lazy object, and the transaction wrapper releases them alongside its own. Same probe after the fix: three timers on build, zero after transaction(). Two other findings from the same round, both correct: - budgetMs went straight into setTimeout, which coerces 0, negatives, NaN and Infinity to a 1ms delay rather than refusing them, so a mistyped queryBudgetMs would have aborted every query almost instantly and read as a total outage. It now throws a RangeError at the call. - The execute probe read a property off an unchecked value, so a driver returning null would crash inside the wrapper and strand the timer. Null is handled and the timer cleared. Also removed the .catch() calls in the laziness test: a lazy Neon query is a thenable, so subscribing to it called execute and fired the very request the test says it never runs.
|
All four correct. Fixed in @chatgpt-codex-connector and @cubic-dev-ai (batch-built query timers): right, and this was self-inflicted. The commit that kept Each build now tags its timer onto the lazy object and the transaction wrapper releases them alongside its own. Test asserts both halves of that probe. Worth stating: fixing the collapse introduced the leak, so this file has now produced the same class of defect twice in a row. Both times it was the interaction with a lazy driver object that the fake-based tests could not see. @coderabbitai (validate @coderabbitai (guard @coderabbitai (the
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
|
To use Codex here, create an environment for this repo. |
@Nicolas0315 Confirmed against the code — all four land the way you describe:
The self-inflicted framing is fair — both defects came from the lazy-object contract hiding behind fake-based tests, and the real-driver probe is what makes that class visible now. Nothing further from me on this round; the N+1-style "same failure twice" note in your reply is the right takeaway to carry forward. One small thing if you touch this file again: the timer tagging via a module-level Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…back form Neon documents two shapes for transaction(): an array of queries, and a callback returning them. The per-statement cleanup added in the previous commit iterated the first argument unconditionally, so the callback form threw 'queries is not iterable' from inside finally, AFTER the request had already succeeded. A working transaction came back as a failure. Reproduced before fixing: transaction(txn => [txn]) threw; both forms now return their result. The first attempt at the guard was itself wrong in a way the test caught: an early inside overwrites the value produced, so the callback form resolved to undefined instead of the driver's result. The cleanup is conditional now rather than early-returning. The internal client type claimed transaction only took an array, which is the lie the wrapper then acted on; it matches the driver's real signature now. Worth noting the type error only surfaced in build and typecheck -- vitest does not typecheck, so the suite was green while this was wrong. Also shortened the budget in the laziness test from 5s to 20ms: that test deliberately never executes the query, so nothing cleared the timer and it added a real 5s tail to the run.
|
All three from this round were right, and the P2 was a genuine crash. Fixed in @cubic-dev-ai (P2, callback form of Both forms return their result now, and there is a regression test for the callback shape. Two things fell out of fixing it that are worth recording:
@cubic-dev-ai (P3, abandoned 5s timer in the laziness test): correct. That test deliberately never executes the query, so nothing clears the timer and it was adding a real 5s tail to the suite. Budget dropped to 20ms; the file now runs in ~50ms. @cubic-dev-ai (P3, two fake-based tests sitting in the driver-contract block): correct — that block's whole purpose is pinning real-driver assumptions and its header comment says so, so putting fake-based cases there undermines the distinction. Will move them; flagging it here rather than silently leaving it. One housekeeping note: the commit message on
|
@Nicolas0315 All three land as you describe, and the callback-form fix looks right in the diff. P2, callback form of P3, abandoned 5s timer: The laziness test now uses a 20ms budget, and the comment explains why (never executes, so nothing clears the timer — a long one would add a real tail). Matches your ~50ms suite figure. P3, fake-based tests in the driver-contract block: Fair call on my part — that block's header explicitly promises real-driver assumptions, and the fake-based cases (invalid budget, callback form, transaction-options, null-return) do blur that line. Moving them keeps the block honest about what it pins. No argument from me; worth doing on the next touch-up. On the mangled commit message: perfectly reasonable to leave it. Nothing further from me on this round — the callback form was the last real defect in that file, and the Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
Four cases exercising withQueryBudget against a fake had drifted into the "driver contract the budget depends on" block, whose stated purpose is pinning what the REAL driver does. That block's header comment draws exactly this distinction, so fake-based cases sitting inside it undermine the separation it exists to make: a reader cannot tell which assertions would survive a driver change. They move back to the withQueryBudget suite. The contract block now holds only the three cases that drive the real Neon client: laziness, the per-statement timers batch leaves behind, and query-level fetchOptions reaching fetch. No assertions changed; 17 tests still pass.
|
@coderabbitai review The three change requests above were addressed across the commits that followed them; the current head is |
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
The default-budget case asserted a range (> 0, <= 30s). The constant's own
comment claims something narrower and load-bearing: the budget does not
exceed `switchActiveTimeoutMs`, so a hung pointer read cannot outlive the
step that issued it. The range never checked that, and would have stayed
green all the way to a 30s default.
Pinning the literal 10_000 instead would only restate the source and would
fail on any deliberate tuning with no bug present. The relationship is the
part worth protecting, so it is asserted against the live policy default
(`fleetPolicySchema.parse({})`) rather than a second hardcoded number.
Verified both directions: raising the default to 10_001 fails with
"expected 10001 to be less than or equal to 10000", while the old range
assertion accepted it.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The guard rejected NaN, Infinity, zero and negatives, but the comment above it names the real hazard: a value setTimeout turns into ~1ms aborts every query and reads as a total outage. Two values did exactly that and passed. A fractional budget (0.5) rounds up to 1ms, and anything past 2^31-1 is clamped to 1ms. `timeoutMsSchema` in @haru/protocol already encodes both bounds, and its comment states the reason. Every other millisecond budget in the repo goes through it; this one did not. It is exported and reused rather than restated, so the two cannot drift. The check also moves from `budgetedOptions` to `withQueryBudget`. The schema comment says the point is that "misconfiguration is a config-time error", and validating per query reported a bad `queryBudgetMs` from whatever statement happened to run first instead of from the wiring that set it. Verified: with the old finite-and-positive check restored, the two added cases fail; the wiring-time assertion holds because the test now calls the constructor rather than a query.
The case wrapped the drizzle call in a bare catch commented "response-
shape mismatch only", but the catch accepted anything. A wrapper that
threw AFTER recording the call would leave the recorded-call assertions
satisfied and the failure invisible, which is the opposite of what this
case exists to prove.
The fake now returns a `fullResults` envelope, so drizzle completes and
the query is simply awaited.
Verified: with the fake rejecting after the call is recorded, the case
now fails ("promise rejected ... instead of resolving"); with the catch
it passed.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Three commits addressing the remaining review comments, plus one they led me to that nobody had flagged.
Each change was checked by breaking it: old guard restored, old range restored, fake made to reject after recording. All gates green locally (build, typecheck, lint, test, format:check). One note for whenever this lands relative to #27: git reports the two as conflict-free, but |
…have The comment said the default budget "does not exceed `switchActiveTimeoutMs`, so a hung pointer read cannot outlive the step that issued it". The relationship holds; the conclusion does not. The step timeout runs from `stepStartedAt`, while this budget restarts at every query, so a read issued partway through a step expires after that step's deadline. With both values at 10_000 there is no margin at all, so only a read issued exactly at step entry is covered. The repo already solves this for the other transport, and the contrast is worth stating rather than papering over: `withSupervisor` caps each call to what remains of the ABSOLUTE `stepDeadlineMs`, which is why `StepContext` documents that "a call issued with 1ms left cannot run for its full per-call timeout on top". Giving the database transport the same property means threading that deadline down into it, which is a larger change than bounding the transport at all, so the comment now states the weaker guarantee it actually provides and names the gap. Assertion unchanged: the budget staying inside the tightest step budget is still worth pinning, and the test comment now describes it as a bound rather than containment.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Want your agent to iterate on Greptile's feedback? Start a greploop in Claude Code and it will work through the open comments and keep going until this PR reviews clean. |
1 participant
What
createDatabasebuilds the Neon client with no fetch options, so every query in the system inherits undici's defaults. This gives each query its ownAbortSignalbudget (DEFAULT_QUERY_BUDGET_MS, 10s, overridable via acreateDatabaseoption rather than a new env knob).Why
This is the one uninstrumented transport in the repo. Heartbeat (5s), the per-nudge supervisor call (10s) and chat TTFB (30s) all carry an explicit budget already; the database did not.
It matters in the mode haru exists to survive. A refused connection fails fast either way, but a store that accepts TCP and never answers made every caller wait on the transport, and the routing pointer is read in two places that care:
cachedSnapshoton the chat hot path, where the wait is paid per request as time-to-first-byte. This is the KNOWN_ISSUES entry "Outage detection latency on the chat path is unbounded".resolveStepTimeoutin the reconciler, where a timed-outswitch_activere-reads the pointer to decide whether routing actually committed. A hang there leaves the operation neither done nor failed. This one was not listed.Two design points worth reviewing
The budget wraps
client.query, not the callable form.drizzle-orm/neon-httpresolvesclient.query ?? clientonce when the session is constructed, and the Neon client does exposequery, soqueryis the only path a repository read or CAS takes. Budgeting the callable form instead compiles, passes its own tests, and never applies to a real query.The signal rides
fetchOptionsrather than racing a timer. Racing a timer against the promise would stop the waiting without cancelling the request, accumulating one abandoned socket per attempt during exactly the outage this is meant to contain. Existing options are merged, not replaced, so drizzle'sarrayMode/fullResults/authTokenand any caller-suppliedfetchOptionssurvive.An expired budget surfaces as a rejection. That is required rather than tolerated:
cachedSnapshotreads a throw from the pointer read as "the store is unreachable", and per AGENTS.md that throw is the only thing licensing a stale route. A budget resolving to a sentinel would defeat the contract silently.Budget sizing
10s matches the supervisor-call budget rather than sitting just above a healthy p99 (a single statement over Neon HTTP is tens of milliseconds): the bound exists to make an outage unmistakable, not to police latency. It also does not exceed
switchActiveTimeoutMs, so a hung pointer read cannot outlive the step that issued it. Happy to change the number if you have a preference.KNOWN_ISSUES
The entry is narrowed rather than deleted, in both languages: the transport half is bounded now, but there is still no memoized outage state, so each request pays the budget again instead of the first one paying it for the rest. That half is a policy decision (how long a "store is down" memo may be trusted, and what clears it) and is left open.
Tests
7 new cases in
packages/db/src/client.test.ts, against a fake client that records what it was handed, so a wrapper missing drizzle's actual call path cannot pass: signal attached to the call drizzle issues, a fresh signal per call, rejection once the budget elapses, a fast query left untouched, drizzle's options and callerfetchOptionspreserved through the merge, other properties passed through, and the default budget bounded.Two cases run on real timers with a 20ms budget:
AbortSignal.timeoutis native and not intercepted by vitest's fake timers, and keeping the production path on a real signal (so the fetch is genuinely cancelled) seemed worth more than making it fakeable.Verification
pnpm test(12/12 tasks),pnpm typecheck,pnpm lint,pnpm format:checkall pass locally on Windows. No schema change, so nodb:generate.Context: I run a small Active/Standby-ish GPU setup of my own and found haru through Hina's Zenn post on serving inference through the GPU shortage, which is also where the ~1s promotion figure sent me looking at what the pointer flip actually depends on. Happy to adjust anything here to taste.
Summary by cubic
Bound every database query and transaction with an abort budget to prevent hangs and surface transport failures quickly. Default is 10s via
DEFAULT_QUERY_BUDGET_MS, configurable viacreateDatabase; budgets compose with caller signals viatimeoutSignal, keep queries lazy and batch-safe, and timers are cleared on settle.Bug Fixes
query()andtransaction; kept queries lazy so batch still works; released per-statement timers in batched queries; handlednulldriver returns and the transaction callback form without crashes or leaks.fetchOptions.signalvia exportedtimeoutSignal.queryBudgetMswith@haru/protocol’s exportedtimeoutMsSchema; reject non‑positive, fractional, NaN/Infinity, and >2^31‑1 at wiring time to avoid 1ms clamp hazards.KNOWN_ISSUES: transport is bounded; outage memoization remains open; clarified docs that the budget is a bound, not a containment guarantee.Tests
fetchOptionsreaching fetch, batch timer release, invalid budgets (including fractional/overflow), andnull-return handling.switchActiveTimeoutMs(fleetPolicySchema); fixed a drizzle integration test to not swallow errors.Written for commit e97e713. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests