From 73c07a02c01f7ce1a48c26fcb4bd14529884f47e Mon Sep 17 00:00:00 2001 From: Max Kuklin <8312264+max-kuklin@users.noreply.github.com> Date: Wed, 3 Sep 2025 21:58:27 +0100 Subject: [PATCH] Revert "Revert "Add query timeout option to cancel long-running queries"" --- cf/src/index.js | 29 +++++++++++++++++------------ cf/src/query.js | 29 ++++++++++++++++++++++++++--- cjs/src/index.js | 29 +++++++++++++++++------------ cjs/src/query.js | 29 ++++++++++++++++++++++++++--- cjs/tests/index.js | 33 +++++++++++++++++++++++++++++++++ deno/src/index.js | 31 ++++++++++++++++++------------- deno/src/query.js | 29 ++++++++++++++++++++++++++--- deno/tests/index.js | 33 +++++++++++++++++++++++++++++++++ deno/types/index.d.ts | 5 +++++ src/index.js | 29 +++++++++++++++++------------ src/query.js | 29 ++++++++++++++++++++++++++--- tests/index.js | 33 +++++++++++++++++++++++++++++++++ types/index.d.ts | 5 +++++ 13 files changed, 282 insertions(+), 61 deletions(-) diff --git a/cf/src/index.js b/cf/src/index.js index 3ffb7e6..c5a68b6 100644 --- a/cf/src/index.js +++ b/cf/src/index.js @@ -110,25 +110,26 @@ function Postgres(a, b) { function sql(strings, ...args) { const query = strings && Array.isArray(strings.raw) - ? new Query(strings, args, handler, cancel) + ? new Query(strings, args, handler, cancel, { postgres_options: options }) : typeof strings === 'string' && !args.length ? new Identifier(options.transform.column.to ? options.transform.column.to(strings) : strings) : new Builder(strings, args) return query } - function unsafe(string, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function unsafe(string, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([string], args, handler, cancel, { prepare: false, - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } - function file(path, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function file(path, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([], args, (query) => { fs.readFile(path, 'utf8', (err, string) => { if (err) @@ -138,8 +139,9 @@ function Postgres(a, b) { handler(query) }) }, cancel, { - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } @@ -357,7 +359,9 @@ function Postgres(a, b) { : ( queries.remove(query), query.cancelled = true, - query.reject(Errors.generic('57014', 'canceling statement due to user request')), + query.reject(Errors.generic('57014', query.timedOut + ? 'canceling statement due to timeout' + : 'canceling statement due to user request')), resolve() ) }) @@ -445,7 +449,7 @@ function parseOptions(a, b) { 'timeout' in o && (console.log('The timeout option is deprecated, use idle_timeout instead'), o.idle_timeout = o.timeout) // eslint-disable-line query.sslrootcert === 'system' && (query.ssl = 'verify-full') - const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive'] + const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive', 'query_timeout'] const defaults = { max : 10, ssl : false, @@ -459,7 +463,8 @@ function parseOptions(a, b) { debug : false, fetch_types : true, publications : 'alltables', - target_session_attrs: null + target_session_attrs: null, + query_timeout : null } return { diff --git a/cf/src/query.js b/cf/src/query.js index 0d44a15..88c83d6 100644 --- a/cf/src/query.js +++ b/cf/src/query.js @@ -23,13 +23,15 @@ export class Query extends Promise { this.state = null this.statement = null - this.resolve = x => (this.active = false, resolve(x)) - this.reject = x => (this.active = false, reject(x)) + this.resolve = x => (this.active = false, this.clearTimeouts(), resolve(x)) + this.reject = x => (this.active = false, this.clearTimeouts(), reject(x)) this.active = false this.cancelled = null this.executed = false this.signature = '' + this.timeoutTimer = null + this.timedOut = false this[originError] = this.handler.debug ? new Error() @@ -50,9 +52,30 @@ export class Query extends Promise { } cancel() { + this.clearTimeouts() return this.canceller && (this.canceller(this), this.canceller = null) } + clearTimeouts() { + if (this.timeoutTimer) { + clearTimeout(this.timeoutTimer) + this.timeoutTimer = null + } + } + + setTimeout() { + const postgresOptions = this.options.postgres_options + if (postgresOptions && postgresOptions.query_timeout && !this.timeoutTimer) { + this.timeoutTimer = setTimeout(() => { + if (this.active && !this.cancelled) { + // Mark as cancelled to distinguish from manual cancellation + this.timedOut = true + this.cancel() + } + }, postgresOptions.query_timeout * 1000) + } + } + simple() { this.options.simple = true this.options.prepare = false @@ -137,7 +160,7 @@ export class Query extends Promise { } async handle() { - !this.executed && (this.executed = true) && await 1 && this.handler(this) + !this.executed && (this.executed = true) && await 1 && (this.setTimeout(), this.handler(this)) } execute() { diff --git a/cjs/src/index.js b/cjs/src/index.js index baf7e60..2780901 100644 --- a/cjs/src/index.js +++ b/cjs/src/index.js @@ -109,25 +109,26 @@ function Postgres(a, b) { function sql(strings, ...args) { const query = strings && Array.isArray(strings.raw) - ? new Query(strings, args, handler, cancel) + ? new Query(strings, args, handler, cancel, { postgres_options: options }) : typeof strings === 'string' && !args.length ? new Identifier(options.transform.column.to ? options.transform.column.to(strings) : strings) : new Builder(strings, args) return query } - function unsafe(string, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function unsafe(string, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([string], args, handler, cancel, { prepare: false, - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } - function file(path, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function file(path, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([], args, (query) => { fs.readFile(path, 'utf8', (err, string) => { if (err) @@ -137,8 +138,9 @@ function Postgres(a, b) { handler(query) }) }, cancel, { - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } @@ -356,7 +358,9 @@ function Postgres(a, b) { : ( queries.remove(query), query.cancelled = true, - query.reject(Errors.generic('57014', 'canceling statement due to user request')), + query.reject(Errors.generic('57014', query.timedOut + ? 'canceling statement due to timeout' + : 'canceling statement due to user request')), resolve() ) }) @@ -444,7 +448,7 @@ function parseOptions(a, b) { 'timeout' in o && (console.log('The timeout option is deprecated, use idle_timeout instead'), o.idle_timeout = o.timeout) // eslint-disable-line query.sslrootcert === 'system' && (query.ssl = 'verify-full') - const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive'] + const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive', 'query_timeout'] const defaults = { max : 10, ssl : false, @@ -458,7 +462,8 @@ function parseOptions(a, b) { debug : false, fetch_types : true, publications : 'alltables', - target_session_attrs: null + target_session_attrs: null, + query_timeout : null } return { diff --git a/cjs/src/query.js b/cjs/src/query.js index 45327f2..f2ced86 100644 --- a/cjs/src/query.js +++ b/cjs/src/query.js @@ -23,13 +23,15 @@ const Query = module.exports.Query = class Query extends Promise { this.state = null this.statement = null - this.resolve = x => (this.active = false, resolve(x)) - this.reject = x => (this.active = false, reject(x)) + this.resolve = x => (this.active = false, this.clearTimeouts(), resolve(x)) + this.reject = x => (this.active = false, this.clearTimeouts(), reject(x)) this.active = false this.cancelled = null this.executed = false this.signature = '' + this.timeoutTimer = null + this.timedOut = false this[originError] = this.handler.debug ? new Error() @@ -50,9 +52,30 @@ const Query = module.exports.Query = class Query extends Promise { } cancel() { + this.clearTimeouts() return this.canceller && (this.canceller(this), this.canceller = null) } + clearTimeouts() { + if (this.timeoutTimer) { + clearTimeout(this.timeoutTimer) + this.timeoutTimer = null + } + } + + setTimeout() { + const postgresOptions = this.options.postgres_options + if (postgresOptions && postgresOptions.query_timeout && !this.timeoutTimer) { + this.timeoutTimer = setTimeout(() => { + if (this.active && !this.cancelled) { + // Mark as cancelled to distinguish from manual cancellation + this.timedOut = true + this.cancel() + } + }, postgresOptions.query_timeout * 1000) + } + } + simple() { this.options.simple = true this.options.prepare = false @@ -137,7 +160,7 @@ const Query = module.exports.Query = class Query extends Promise { } async handle() { - !this.executed && (this.executed = true) && await 1 && this.handler(this) + !this.executed && (this.executed = true) && await 1 && (this.setTimeout(), this.handler(this)) } execute() { diff --git a/cjs/tests/index.js b/cjs/tests/index.js index ec5222f..3d55b01 100644 --- a/cjs/tests/index.js +++ b/cjs/tests/index.js @@ -2614,3 +2614,36 @@ t('Ensure reserve on query throws proper error', async() => { 'wat', x, reserved.release() ] }) + +t('Query timeout cancels long running query', async() => { + const sql = postgres({ + ...options, + query_timeout: 1 + }) + + const start = Date.now() + const error = await sql`select pg_sleep(3)`.catch(e => e) + const elapsed = Date.now() - start + + return [ + '57014', + error.code, + elapsed < 2000, // Should timeout in ~1 second, not 3 seconds + await sql.end() + ] +}) + +t('Query timeout allows quick queries to complete', async() => { + const sql = postgres({ + ...options, + query_timeout: 2 + }) + + const result = await sql`select 1 as x` + + return [ + 1, + result[0].x, + await sql.end() + ] +}) diff --git a/deno/src/index.js b/deno/src/index.js index aa7a920..52e61f0 100644 --- a/deno/src/index.js +++ b/deno/src/index.js @@ -110,25 +110,26 @@ function Postgres(a, b) { function sql(strings, ...args) { const query = strings && Array.isArray(strings.raw) - ? new Query(strings, args, handler, cancel) + ? new Query(strings, args, handler, cancel, { postgres_options: options }) : typeof strings === 'string' && !args.length ? new Identifier(options.transform.column.to ? options.transform.column.to(strings) : strings) : new Builder(strings, args) return query } - function unsafe(string, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function unsafe(string, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([string], args, handler, cancel, { prepare: false, - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } - function file(path, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function file(path, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([], args, (query) => { fs.readFile(path, 'utf8', (err, string) => { if (err) @@ -138,8 +139,9 @@ function Postgres(a, b) { handler(query) }) }, cancel, { - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } @@ -357,7 +359,9 @@ function Postgres(a, b) { : ( queries.remove(query), query.cancelled = true, - query.reject(Errors.generic('57014', 'canceling statement due to user request')), + query.reject(Errors.generic('57014', query.timedOut + ? 'canceling statement due to timeout' + : 'canceling statement due to user request')), resolve() ) }) @@ -445,7 +449,7 @@ function parseOptions(a, b) { 'timeout' in o && (console.log('The timeout option is deprecated, use idle_timeout instead'), o.idle_timeout = o.timeout) // eslint-disable-line query.sslrootcert === 'system' && (query.ssl = 'verify-full') - const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive'] + const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive', 'query_timeout'] const defaults = { max : 10, ssl : false, @@ -459,7 +463,8 @@ function parseOptions(a, b) { debug : false, fetch_types : true, publications : 'alltables', - target_session_attrs: null + target_session_attrs: null, + query_timeout : null } return { @@ -482,8 +487,8 @@ function parseOptions(a, b) { {} ), connection : { + application_name: env.PGAPPNAME || 'postgres.js', ...o.connection, - application_name: o.connection?.application_name ?? env.PGAPPNAME ?? 'postgres.js', ...Object.entries(query).reduce((acc, [k, v]) => (k in defaults || (acc[k] = v), acc), {}) }, types : o.types || {}, diff --git a/deno/src/query.js b/deno/src/query.js index 0d44a15..88c83d6 100644 --- a/deno/src/query.js +++ b/deno/src/query.js @@ -23,13 +23,15 @@ export class Query extends Promise { this.state = null this.statement = null - this.resolve = x => (this.active = false, resolve(x)) - this.reject = x => (this.active = false, reject(x)) + this.resolve = x => (this.active = false, this.clearTimeouts(), resolve(x)) + this.reject = x => (this.active = false, this.clearTimeouts(), reject(x)) this.active = false this.cancelled = null this.executed = false this.signature = '' + this.timeoutTimer = null + this.timedOut = false this[originError] = this.handler.debug ? new Error() @@ -50,9 +52,30 @@ export class Query extends Promise { } cancel() { + this.clearTimeouts() return this.canceller && (this.canceller(this), this.canceller = null) } + clearTimeouts() { + if (this.timeoutTimer) { + clearTimeout(this.timeoutTimer) + this.timeoutTimer = null + } + } + + setTimeout() { + const postgresOptions = this.options.postgres_options + if (postgresOptions && postgresOptions.query_timeout && !this.timeoutTimer) { + this.timeoutTimer = setTimeout(() => { + if (this.active && !this.cancelled) { + // Mark as cancelled to distinguish from manual cancellation + this.timedOut = true + this.cancel() + } + }, postgresOptions.query_timeout * 1000) + } + } + simple() { this.options.simple = true this.options.prepare = false @@ -137,7 +160,7 @@ export class Query extends Promise { } async handle() { - !this.executed && (this.executed = true) && await 1 && this.handler(this) + !this.executed && (this.executed = true) && await 1 && (this.setTimeout(), this.handler(this)) } execute() { diff --git a/deno/tests/index.js b/deno/tests/index.js index adedf1e..b566fbe 100644 --- a/deno/tests/index.js +++ b/deno/tests/index.js @@ -2617,4 +2617,37 @@ t('Ensure reserve on query throws proper error', async() => { ] }) +t('Query timeout cancels long running query', async() => { + const sql = postgres({ + ...options, + query_timeout: 1 + }) + + const start = Date.now() + const error = await sql`select pg_sleep(3)`.catch(e => e) + const elapsed = Date.now() - start + + return [ + '57014', + error.code, + elapsed < 2000, // Should timeout in ~1 second, not 3 seconds + await sql.end() + ] +}) + +t('Query timeout allows quick queries to complete', async() => { + const sql = postgres({ + ...options, + query_timeout: 2 + }) + + const result = await sql`select 1 as x` + + return [ + 1, + result[0].x, + await sql.end() + ] +}) + ;globalThis.addEventListener("unload", () => Deno.exit(process.exitCode)) \ No newline at end of file diff --git a/deno/types/index.d.ts b/deno/types/index.d.ts index 44a07af..ceaec7d 100644 --- a/deno/types/index.d.ts +++ b/deno/types/index.d.ts @@ -62,6 +62,11 @@ interface BaseOptions> { * @default process.env['PGCONNECT_TIMEOUT'] */ connect_timeout: number; + /** + * Query timeout in seconds - cancels queries that run longer than this + * @default null (no timeout) + */ + query_timeout: number | null | undefined; /** Array of custom types; see more in the README */ types: T; /** diff --git a/src/index.js b/src/index.js index 944d50c..c495610 100644 --- a/src/index.js +++ b/src/index.js @@ -109,25 +109,26 @@ function Postgres(a, b) { function sql(strings, ...args) { const query = strings && Array.isArray(strings.raw) - ? new Query(strings, args, handler, cancel) + ? new Query(strings, args, handler, cancel, { postgres_options: options }) : typeof strings === 'string' && !args.length ? new Identifier(options.transform.column.to ? options.transform.column.to(strings) : strings) : new Builder(strings, args) return query } - function unsafe(string, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function unsafe(string, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([string], args, handler, cancel, { prepare: false, - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } - function file(path, args = [], options = {}) { - arguments.length === 2 && !Array.isArray(args) && (options = args, args = []) + function file(path, args = [], queryOptions = {}) { + arguments.length === 2 && !Array.isArray(args) && (queryOptions = args, args = []) const query = new Query([], args, (query) => { fs.readFile(path, 'utf8', (err, string) => { if (err) @@ -137,8 +138,9 @@ function Postgres(a, b) { handler(query) }) }, cancel, { - ...options, - simple: 'simple' in options ? options.simple : args.length === 0 + ...queryOptions, + simple: 'simple' in queryOptions ? queryOptions.simple : args.length === 0, + postgres_options: options }) return query } @@ -356,7 +358,9 @@ function Postgres(a, b) { : ( queries.remove(query), query.cancelled = true, - query.reject(Errors.generic('57014', 'canceling statement due to user request')), + query.reject(Errors.generic('57014', query.timedOut + ? 'canceling statement due to timeout' + : 'canceling statement due to user request')), resolve() ) }) @@ -444,7 +448,7 @@ function parseOptions(a, b) { 'timeout' in o && (console.log('The timeout option is deprecated, use idle_timeout instead'), o.idle_timeout = o.timeout) // eslint-disable-line query.sslrootcert === 'system' && (query.ssl = 'verify-full') - const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive'] + const ints = ['idle_timeout', 'connect_timeout', 'max_lifetime', 'max_pipeline', 'backoff', 'keep_alive', 'query_timeout'] const defaults = { max : 10, ssl : false, @@ -458,7 +462,8 @@ function parseOptions(a, b) { debug : false, fetch_types : true, publications : 'alltables', - target_session_attrs: null + target_session_attrs: null, + query_timeout : null } return { diff --git a/src/query.js b/src/query.js index 0d44a15..88c83d6 100644 --- a/src/query.js +++ b/src/query.js @@ -23,13 +23,15 @@ export class Query extends Promise { this.state = null this.statement = null - this.resolve = x => (this.active = false, resolve(x)) - this.reject = x => (this.active = false, reject(x)) + this.resolve = x => (this.active = false, this.clearTimeouts(), resolve(x)) + this.reject = x => (this.active = false, this.clearTimeouts(), reject(x)) this.active = false this.cancelled = null this.executed = false this.signature = '' + this.timeoutTimer = null + this.timedOut = false this[originError] = this.handler.debug ? new Error() @@ -50,9 +52,30 @@ export class Query extends Promise { } cancel() { + this.clearTimeouts() return this.canceller && (this.canceller(this), this.canceller = null) } + clearTimeouts() { + if (this.timeoutTimer) { + clearTimeout(this.timeoutTimer) + this.timeoutTimer = null + } + } + + setTimeout() { + const postgresOptions = this.options.postgres_options + if (postgresOptions && postgresOptions.query_timeout && !this.timeoutTimer) { + this.timeoutTimer = setTimeout(() => { + if (this.active && !this.cancelled) { + // Mark as cancelled to distinguish from manual cancellation + this.timedOut = true + this.cancel() + } + }, postgresOptions.query_timeout * 1000) + } + } + simple() { this.options.simple = true this.options.prepare = false @@ -137,7 +160,7 @@ export class Query extends Promise { } async handle() { - !this.executed && (this.executed = true) && await 1 && this.handler(this) + !this.executed && (this.executed = true) && await 1 && (this.setTimeout(), this.handler(this)) } execute() { diff --git a/tests/index.js b/tests/index.js index 07ff98e..da223aa 100644 --- a/tests/index.js +++ b/tests/index.js @@ -2614,3 +2614,36 @@ t('Ensure reserve on query throws proper error', async() => { 'wat', x, reserved.release() ] }) + +t('Query timeout cancels long running query', async() => { + const sql = postgres({ + ...options, + query_timeout: 1 + }) + + const start = Date.now() + const error = await sql`select pg_sleep(3)`.catch(e => e) + const elapsed = Date.now() - start + + return [ + '57014', + error.code, + elapsed < 2000, // Should timeout in ~1 second, not 3 seconds + await sql.end() + ] +}) + +t('Query timeout allows quick queries to complete', async() => { + const sql = postgres({ + ...options, + query_timeout: 2 + }) + + const result = await sql`select 1 as x` + + return [ + 1, + result[0].x, + await sql.end() + ] +}) diff --git a/types/index.d.ts b/types/index.d.ts index eb60491..a499d97 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -60,6 +60,11 @@ interface BaseOptions> { * @default process.env['PGCONNECT_TIMEOUT'] */ connect_timeout: number; + /** + * Query timeout in seconds - cancels queries that run longer than this + * @default null (no timeout) + */ + query_timeout: number | null | undefined; /** Array of custom types; see more in the README */ types: T; /**