Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace Bluebird.timeout #3634

Merged
merged 5 commits into from Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;