Skip to content

feat: add ability to set per-request requestTimeout#1530

Merged
dhensby merged 2 commits intotediousjs:masterfrom
dhensby:pulls/per-req-timeout
Apr 23, 2026
Merged

feat: add ability to set per-request requestTimeout#1530
dhensby merged 2 commits intotediousjs:masterfrom
dhensby:pulls/per-req-timeout

Conversation

@dhensby
Copy link
Copy Markdown
Collaborator

@dhensby dhensby commented Aug 16, 2023

This adds an optional overrides object that can be passed to Request, Transaction and PreparedStatement objects to provide per-instance config overrides. Currently this supports requestTimeout, but the pattern is extensible for future options.

Usage

// Per-request timeout
const request = new sql.Request(pool, { requestTimeout: 60000 })

// Transaction with timeout that cascades to child requests
const transaction = new sql.Transaction(pool, { requestTimeout: 60000 })
const req1 = transaction.request() // inherits 60s timeout
const req2 = transaction.request({ requestTimeout: 3000 }) // override to 3s

// Prepared statement with timeout
const ps = new sql.PreparedStatement(pool, { requestTimeout: 60000 })

Changes

  • Request, Transaction, PreparedStatement constructors accept an optional overrides object
  • Transaction overrides cascade to child requests (merged at property level, so per-request overrides take precedence)
  • PreparedStatement overrides are applied to internal prepare, execute, and unprepare requests
  • Timeout is set via req.setTimeout() for tedious and query_timeout for msnodesqlv8
  • Validates that requestTimeout is a finite non-negative number (rejects NaN, Infinity, negatives)
  • Covers all tedious code paths: _query, _bulk, and _execute (stored procedures)
  • Transaction timeout applies to data requests only, not begin/commit/rollback operations

Notes

  • The transaction begin/commit/rollback timeout is not overridable because tedious creates internal requests for these operations without exposing them
  • requestTimeout: 0 is accepted as a valid value (meaning no timeout)

Closes #1529

@dhensby dhensby force-pushed the pulls/per-req-timeout branch from f48d603 to b919a7e Compare April 14, 2026 23:32
@dhensby dhensby marked this pull request as ready for review April 14, 2026 23:38
@dhensby dhensby force-pushed the pulls/per-req-timeout branch 2 times, most recently from 8820e1a to a7d303c Compare April 15, 2026 23:00
@dhensby dhensby requested a review from Copilot April 15, 2026 23:01
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/options objects to Request, Transaction, and PreparedStatement constructors; transaction overrides cascade to child requests.
  • Apply per-request timeout in tedious via req.setTimeout() and in msnodesqlv8 via query_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.

Comment thread lib/base/transaction.js Outdated
Comment thread lib/base/transaction.js
Comment thread lib/base/prepared-statement.js
Comment thread lib/base/connection-pool.js
Comment thread lib/base/connection-pool.js
Comment thread test/common/tests.js Outdated
Comment thread lib/msnodesqlv8/request.js Outdated
Comment thread lib/base/request.js Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and PreparedStatement constructors (and pool factory methods) to accept per-instance overrides (currently requestTimeout).
  • Apply the per-request timeout in both drivers (tedious via req.setTimeout(), msnodesqlv8 via query_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.

Comment thread lib/base/transaction.js
Comment thread lib/base/transaction.js Outdated
Comment thread test/common/tests.js
Comment thread test/common/tests.js
Comment thread test/msnodesqlv8/msnodesqlv8.js
Comment thread lib/msnodesqlv8/request.js
@dhensby dhensby force-pushed the pulls/per-req-timeout branch 2 times, most recently from 4bf9cc7 to 074156b Compare April 22, 2026 22:40
@dhensby dhensby requested a review from Copilot April 22, 2026 22:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 overrides objects on Request/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.

Comment thread test/common/tests.js
Comment thread test/common/tests.js
Comment thread test/common/tests.js Outdated
Comment thread test/common/tests.js
Comment thread .releaserc Outdated
Comment thread README.md
dhensby and others added 2 commits April 23, 2026 01:19
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>
@dhensby dhensby force-pushed the pulls/per-req-timeout branch from 7ce9e04 to 3b1645f Compare April 23, 2026 00:19
@dhensby dhensby merged commit 7a8dcd4 into tediousjs:master Apr 23, 2026
47 checks passed
@dhensby dhensby deleted the pulls/per-req-timeout branch April 23, 2026 06:57
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 12.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Request.prototype.setTimeout()

3 participants