feat: add ability to set per-request requestTimeout#1530
feat: add ability to set per-request requestTimeout#1530dhensby merged 2 commits intotediousjs:masterfrom
Conversation
f48d603 to
b919a7e
Compare
8820e1a to
a7d303c
Compare
There was a problem hiding this comment.
Pull request overview
Adds a driver-agnostic mechanism for per-instance overrides (currently requestTimeout) on Request, Transaction, and PreparedStatement, so callers can override the pool default timeout without creating multiple pools. This fits into the existing base/driver layering by storing overrides in base classes and applying them in tedious/msnodesqlv8 driver request execution paths.
Changes:
- Add optional
overrides/optionsobjects toRequest,Transaction, andPreparedStatementconstructors; transaction overrides cascade to child requests. - Apply per-request timeout in tedious via
req.setTimeout()and in msnodesqlv8 viaquery_timeout. - Add unit + integration tests and update docs/SQL fixtures for the new behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
lib/base/request.js |
Stores validated per-request overrides (e.g. requestTimeout). |
lib/base/transaction.js |
Stores transaction overrides and merges them into Transaction#request() calls. |
lib/base/prepared-statement.js |
Stores statement overrides and applies them to internal prepare/execute/unprepare requests. |
lib/base/connection-pool.js |
Allows passing overrides through pool.request(conf) / pool.transaction(conf). |
lib/tedious/request.js |
Applies per-request timeout to tedious Request instances across query/bulk/execute code paths. |
lib/tedious/transaction.js |
Forwards overrides to the base Transaction constructor. |
lib/msnodesqlv8/request.js |
Applies per-request timeout by setting query_timeout for msnodesqlv8 queries (including bulk setup). |
test/common/unit.js |
Adds unit coverage for validation/storage/override-cascading behavior in base classes. |
test/common/tests.js |
Adds integration tests validating per-request timeout behavior across scenarios. |
test/msnodesqlv8/msnodesqlv8.js |
Registers the new integration test cases for msnodesqlv8 runs. |
test/prepare.sql / test/cleanup.sql |
Adds/removes a delayed stored procedure used for timeout testing. |
README.md |
Documents the new optional per-instance options argument and its semantics. |
There was a problem hiding this comment.
Pull request overview
Adds a per-instance “overrides/options” object to core API objects so callers can set requestTimeout on a Request, Transaction (cascading to child requests), and PreparedStatement (applied to internal requests), avoiding the need for multiple pools.
Changes:
- Extend
Request,Transaction, andPreparedStatementconstructors (and pool factory methods) to accept per-instance overrides (currentlyrequestTimeout). - Apply the per-request timeout in both drivers (
tediousviareq.setTimeout(),msnodesqlv8viaquery_timeout). - Add unit + integration tests and update docs to describe the new usage.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/prepare.sql | Adds __testDelay stored proc used for timeout integration tests. |
| test/cleanup.sql | Drops __testDelay during test cleanup. |
| test/msnodesqlv8/msnodesqlv8.js | Wires new per-request timeout tests into the msnodesqlv8 runner. |
| test/common/unit.js | Adds unit tests for override validation/storage and transaction cascading behavior. |
| test/common/tests.js | Adds integration tests covering per-request timeout behavior (pool override, isolation between requests, tx cascade, sproc, prepared statement). |
| lib/tedious/transaction.js | Plumbs overrides through tedious Transaction constructor. |
| lib/tedious/request.js | Applies per-request timeout via tedious.Request#setTimeout() across bulk/query/execute paths. |
| lib/msnodesqlv8/request.js | Applies per-request timeout via query_timeout for query/bulk preflight SQL. |
| lib/base/transaction.js | Stores overrides, adds Transaction.request(config) to merge/cascade overrides. |
| lib/base/request.js | Stores validated per-request overrides on Request. |
| lib/base/prepared-statement.js | Stores overrides and applies them to internal prepare/execute/unprepare requests. |
| lib/base/connection-pool.js | Allows passing per-instance overrides through pool.request(conf) / pool.transaction(conf). |
| README.md | Documents the new optional options argument and behavior for Request/Transaction/PreparedStatement. |
4bf9cc7 to
074156b
Compare
There was a problem hiding this comment.
Pull request overview
Adds an extensible “overrides” mechanism to allow per-instance configuration—starting with requestTimeout—across Request, Transaction, and PreparedStatement, with driver-specific wiring for tedious and msnodesqlv8.
Changes:
- Introduces optional
overridesobjects onRequest/Transaction/PreparedStatement, with validation and transaction-level inheritance to child requests. - Applies per-request timeouts in tedious (
req.setTimeout(...)) and msnodesqlv8 (query_timeout). - Adds unit/integration tests plus docs for the new per-request timeout behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/base/request.js | Stores validated per-request overrides (currently requestTimeout). |
| lib/base/transaction.js | Stores transaction overrides and cascades them to transaction.request(...). |
| lib/base/prepared-statement.js | Stores statement overrides and applies them to internal prepare/execute/unprepare requests. |
| lib/base/connection-pool.js | Allows pool.request(conf) / pool.transaction(conf) to pass override objects through. |
| lib/tedious/request.js | Applies override timeout via tedious.Request#setTimeout() for query/execute/bulk-create paths. |
| lib/tedious/transaction.js | Threads overrides into the tedious Transaction constructor. |
| lib/msnodesqlv8/request.js | Applies override timeout via query_timeout for query/bulk-create paths. |
| test/common/unit.js | Adds unit tests validating override storage/validation and transaction cascade semantics. |
| test/common/tests.js | Adds integration tests for per-request timeout across requests/transactions/SPs/prepared statements. |
| test/tedious/tedious.js | Wires new per-request timeout integration tests into tedious runner. |
| test/msnodesqlv8/msnodesqlv8.js | Wires new per-request timeout integration tests into msnodesqlv8 runner. |
| test/prepare.sql | Adds __testDelay stored procedure used by timeout integration tests. |
| test/cleanup.sql | Drops __testDelay during test cleanup. |
| README.md | Documents the new optional [options] argument and requestTimeout override behavior. |
| .releaserc | Adds @semantic-release/git plugin to release pipeline config. |
Add an optional overrides object to Request, Transaction and PreparedStatement constructors for per-instance config overrides. Currently supports requestTimeout. - Transaction overrides cascade to child requests unless overridden - PreparedStatement overrides apply to prepare, execute and unprepare - Validates timeout is a finite non-negative number - Transaction.request() validates per-request config before merging - Fixes missing setTimeout in tedious stored procedure path (_execute) - requestTimeout does not apply to bulk insert operations Closes tediousjs#1529 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add functional tests to verify per-request requestTimeout works across all code paths: query, stored procedure, transaction, and prepared statement. Tests confirm the timeout override fires independently of the pool-level default and that unrelated requests are not affected. Wired into both tedious and msnodesqlv8 test runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7ce9e04 to
3b1645f
Compare
|
🎉 This PR is included in version 12.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This adds an optional overrides object that can be passed to
Request,TransactionandPreparedStatementobjects to provide per-instance config overrides. Currently this supportsrequestTimeout, but the pattern is extensible for future options.Usage
Changes
Request,Transaction,PreparedStatementconstructors accept an optionaloverridesobjectprepare,execute, andunpreparerequestsreq.setTimeout()for tedious andquery_timeoutfor msnodesqlv8requestTimeoutis a finite non-negative number (rejectsNaN,Infinity, negatives)_query,_bulk, and_execute(stored procedures)begin/commit/rollbackoperationsNotes
begin/commit/rollbacktimeout is not overridable because tedious creates internal requests for these operations without exposing themrequestTimeout: 0is accepted as a valid value (meaning no timeout)Closes #1529