Skip to content

Commit

Permalink
replace Bluebird.timeout (#3634)
Browse files Browse the repository at this point in the history
  • Loading branch information
maximelkin committed Feb 12, 2020
1 parent 92d3944 commit 88d832c
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 38 deletions.
15 changes: 10 additions & 5 deletions lib/client.js
Expand Up @@ -25,6 +25,7 @@ const { makeEscape } = require('./query/string');
const { uniqueId, cloneDeep, defaults } = require('lodash');

const Logger = require('./logger');
const { KnexTimeoutError } = require('./util/timeout');

const debug = require('debug')('knex:client');
const _debugQuery = require('debug')('knex:query');
Expand Down Expand Up @@ -344,11 +345,15 @@ Object.assign(Client.prototype, {
debug('acquired connection from pool: %s', connection.__knexUid);
return connection;
})
.catch(TimeoutError, () => {
throw new Bluebird.TimeoutError(
'Knex: Timeout acquiring a connection. The pool is probably full. ' +
'Are you missing a .transacting(trx) call?'
);
.catch((error) => {
let convertedError = error;
if (error instanceof TimeoutError) {
convertedError = new KnexTimeoutError(
'Knex: Timeout acquiring a connection. The pool is probably full. ' +
'Are you missing a .transacting(trx) call?'
);
}
throw convertedError;
});
} catch (e) {
return Bluebird.reject(e);
Expand Down
4 changes: 2 additions & 2 deletions lib/dialects/mysql/index.js
Expand Up @@ -4,6 +4,7 @@ const inherits = require('inherits');
const { map, defer } = require('lodash');
const { promisify } = require('util');
const Client = require('../../client');
const { timeout } = require('../../util/timeout');
const Bluebird = require('bluebird');

const Transaction = require('./transaction');
Expand Down Expand Up @@ -177,8 +178,7 @@ Object.assign(Client_MySQL.prototype, {
// Purposely not putting timeout on `KILL QUERY` execution because erroring
// early there would release the `connectionToKill` back to the pool with
// a `KILL QUERY` command yet to finish.
return acquiringConn
.timeout(100)
return timeout(acquiringConn, 100)
.then((conn) =>
this.query(conn, {
method: 'raw',
Expand Down
21 changes: 11 additions & 10 deletions lib/dialects/oracledb/transaction.js
Expand Up @@ -2,6 +2,7 @@ const { isUndefined } = require('lodash');

const Bluebird = require('bluebird');
const Transaction = require('../../transaction');
const { timeout, KnexTimeoutError } = require('../../util/timeout');
const debugTx = require('debug')('knex:tx');

module.exports = class Oracle_Transaction extends Transaction {
Expand All @@ -23,24 +24,24 @@ module.exports = class Oracle_Transaction extends Transaction {
}

rollback(conn, err) {
const self = this;
this._completed = true;
debugTx('%s: rolling back', this.txid);
return conn
.rollbackAsync()
.timeout(5000)
.catch(Bluebird.TimeoutError, function(e) {
self._rejecter(e);
return timeout(conn.rollbackAsync(), 5000)
.catch((e) => {
if (!(e instanceof KnexTimeoutError)) {
return Promise.reject(e);
}
this._rejecter(e);
})
.then(function() {
.then(() => {
if (isUndefined(err)) {
if (self.doNotRejectOnRollback) {
self._resolver();
if (this.doNotRejectOnRollback) {
this._resolver();
return;
}
err = new Error(`Transaction rejected with non-error: ${err}`);
}
self._rejecter(err);
this._rejecter(err);
});
}

Expand Down
4 changes: 4 additions & 0 deletions lib/knex.js
Expand Up @@ -5,6 +5,7 @@ const QueryInterface = require('./query/methods');

const makeKnex = require('./util/make-knex');
const parseConnection = require('./util/parse-connection');
const { KnexTimeoutError } = require('./util/timeout');
const fakeClient = require('./util/fake-client');
const { SUPPORTED_CLIENTS } = require('./constants');
const { resolveClientNameWithAliases } = require('./helpers');
Expand Down Expand Up @@ -58,6 +59,9 @@ function Knex(config) {

// Expose Client on the main Knex namespace.
Knex.Client = Client;

Knex.KnexTimeoutError = KnexTimeoutError;

Knex.QueryBuilder = {
extend: function(methodName, fn) {
QueryBuilder.extend(methodName, fn);
Expand Down
14 changes: 11 additions & 3 deletions lib/runner.js
@@ -1,4 +1,6 @@
const Bluebird = require('bluebird');
const { KnexTimeoutError } = require('./util/timeout');
const { timeout } = require('./util/timeout');

let PassThrough;

Expand Down Expand Up @@ -133,7 +135,7 @@ Object.assign(Runner.prototype, {
let queryPromise = this.client.query(this.connection, obj);

if (obj.timeout) {
queryPromise = queryPromise.timeout(obj.timeout);
queryPromise = timeout(queryPromise, obj.timeout);
}

// Await the return value of client.processResponse; in the case of sqlite3's
Expand Down Expand Up @@ -164,7 +166,10 @@ Object.assign(Runner.prototype, {

return postProcessedResponse;
})
.catch(Bluebird.TimeoutError, (error) => {
.catch((error) => {
if (!(error instanceof KnexTimeoutError)) {
return Promise.reject(error);
}
const { timeout, sql, bindings } = obj;

let cancelQuery;
Expand Down Expand Up @@ -241,7 +246,10 @@ Object.assign(Runner.prototype, {
}
return this.client
.acquireConnection()
.catch(Bluebird.TimeoutError, (error) => {
.catch((error) => {
if (!(error instanceof KnexTimeoutError)) {
return Promise.reject(error);
}
if (this.builder) {
error.sql = this.builder.sql;
error.bindings = this.builder.bindings;
Expand Down
26 changes: 17 additions & 9 deletions lib/transaction.js
Expand Up @@ -5,6 +5,7 @@ const { EventEmitter } = require('events');
const Debug = require('debug');

const makeKnex = require('./util/make-knex');
const { timeout, KnexTimeoutError } = require('./util/timeout');

const debug = Debug('knex:tx');

Expand Down Expand Up @@ -150,19 +151,26 @@ class Transaction extends EventEmitter {
}

rollback(conn, error) {
return this.query(conn, 'ROLLBACK', 2, error)
.timeout(5000)
.catch(Bluebird.TimeoutError, () => {
return timeout(this.query(conn, 'ROLLBACK', 2, error), 5000).catch(
(err) => {
if (!(err instanceof KnexTimeoutError)) {
return Promise.reject(err);
}
this._rejecter(error);
});
}
);
}

rollbackTo(conn, error) {
return this.query(conn, `ROLLBACK TO SAVEPOINT ${this.txid}`, 2, error)
.timeout(5000)
.catch(Bluebird.TimeoutError, () => {
this._rejecter(error);
});
return timeout(
this.query(conn, `ROLLBACK TO SAVEPOINT ${this.txid}`, 2, error),
5000
).catch((err) => {
if (!(err instanceof KnexTimeoutError)) {
return Promise.reject(err);
}
this._rejecter(error);
});
}

query(conn, sql, status, value) {
Expand Down
20 changes: 20 additions & 0 deletions lib/util/timeout.js
@@ -0,0 +1,20 @@
const Bluebird = require('bluebird');
const delay = require('./delay');

class KnexTimeoutError extends Error {
constructor(message) {
super(message);
this.name = 'KnexTimeoutError';
}
}

module.exports.KnexTimeoutError = KnexTimeoutError;
module.exports.timeout = (promise, ms) =>
Bluebird.resolve(
Promise.race([
promise,
delay(ms).then(() =>
Promise.reject(new KnexTimeoutError('operation timed out'))
),
])
);
8 changes: 4 additions & 4 deletions test/integration/builder/additional.js
Expand Up @@ -739,7 +739,7 @@ module.exports = function(knex) {
.catch(function(error) {
expect(_.pick(error, 'timeout', 'name', 'message')).to.deep.equal({
timeout: 200,
name: 'TimeoutError',
name: 'KnexTimeoutError',
message:
'Defined query timeout of 200ms exceeded when running query.',
});
Expand Down Expand Up @@ -826,7 +826,7 @@ module.exports = function(knex) {
.catch(function(error) {
expect(_.pick(error, 'timeout', 'name', 'message')).to.deep.equal({
timeout: 200,
name: 'TimeoutError',
name: 'KnexTimeoutError',
message:
'Defined query timeout of 200ms exceeded when running query.',
});
Expand Down Expand Up @@ -911,7 +911,7 @@ module.exports = function(knex) {
.catch(function(error) {
expect(_.pick(error, 'timeout', 'name', 'message')).to.deep.equal({
timeout: 1,
name: 'TimeoutError',
name: 'KnexTimeoutError',
message:
'After query timeout of 1ms exceeded, cancelling of query failed.',
});
Expand Down Expand Up @@ -980,7 +980,7 @@ module.exports = function(knex) {
.catch(function(error) {
expect(_.pick(error, 'timeout', 'name', 'message')).to.deep.equal({
timeout: secondQueryTimeout,
name: 'TimeoutError',
name: 'KnexTimeoutError',
message: `Defined query timeout of ${secondQueryTimeout}ms exceeded when running query.`,
});
})
Expand Down
3 changes: 2 additions & 1 deletion test/integration/builder/transaction.js
Expand Up @@ -6,6 +6,7 @@ const Bluebird = require('bluebird');
const Knex = require('../../../knex');
const _ = require('lodash');
const sinon = require('sinon');
const { KnexTimeoutError } = require('../../../lib/util/timeout');

module.exports = function(knex) {
// Certain dialects do not have proper insert with returning, so if this is true
Expand Down Expand Up @@ -505,7 +506,7 @@ module.exports = function(knex) {
.then(function() {
throw new Error('should not get here');
})
.catch(Bluebird.TimeoutError, function(error) {});
.catch(KnexTimeoutError, function(error) {});
});

/**
Expand Down
8 changes: 4 additions & 4 deletions test/tape/invalid-db-setup.js
Expand Up @@ -3,7 +3,7 @@
const tape = require('tape');
const _ = require('lodash');
const makeKnex = require('../../knex');
const Bluebird = require('bluebird');
const { KnexTimeoutError } = require('../../lib/util/timeout');

module.exports = (knexfile) => {
Object.keys(knexfile).forEach((key) => {
Expand All @@ -29,7 +29,7 @@ module.exports = (knexfile) => {
.then((res) => {
t.fail(`Query should have failed, got: ${JSON.stringify(res)}`);
})
.catch(Bluebird.TimeoutError, (e) => {
.catch(KnexTimeoutError, (e) => {
t.fail(`Query should have failed with non timeout error`);
})
.catch((e) => {
Expand All @@ -54,7 +54,7 @@ module.exports = (knexfile) => {
`Stream query should have failed, got: ${JSON.stringify(res)}`
);
})
.catch(Bluebird.TimeoutError, (e) => {
.catch(KnexTimeoutError, (e) => {
t.fail(`Stream query should have failed with non timeout error`);
})
.catch((e) => {
Expand Down Expand Up @@ -104,7 +104,7 @@ module.exports = (knexfile) => {
.then(() => {
t.fail('query should have stalled');
})
.catch(Bluebird.TimeoutError, (e) => {
.catch(KnexTimeoutError, (e) => {
t.pass('Got acquireTimeout error');
})
.catch((e) => {
Expand Down
2 changes: 2 additions & 0 deletions types/index.d.ts
Expand Up @@ -1990,6 +1990,8 @@ declare namespace Knex {
) => QueryBuilder<TRecord, TResult>
): void;
}

export class KnexTimeoutError extends Error {}
}

export = Knex;

0 comments on commit 88d832c

Please sign in to comment.