Skip to content

Commit

Permalink
fix: do not set a cancellation timer if request is cancelled before i…
Browse files Browse the repository at this point in the history
…t is fully sent off

This fixes an issue where if a request or bulk request was cancelled while it was still being sent, the connection would be destroyed after the cancellation timeout period.
  • Loading branch information
arthurschreiber authored Mar 7, 2021
1 parent eaf297f commit 37f9084
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 3 deletions.
3 changes: 0 additions & 3 deletions src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3094,9 +3094,6 @@ class Connection extends EventEmitter {
message.ignore = true;
message.end();

this.clearRequestTimer();
this.createCancelTimer();

if (request instanceof Request && request.paused) {
// resume the request if it was paused so we can read the remaining tokens
request.resume();
Expand Down
54 changes: 54 additions & 0 deletions test/integration/bulk-load-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,60 @@ describe('BulkLoad', function() {
}
});

it('should not close the connection due to cancelTimeout if streaming bulk load is cancelled', function(done) {
const totalRows = 20;

const sql = 'create table #stream_test (i int not null primary key)';
const request = new Request(sql, (err) => {
if (err) {
return done(err);
}

const bulkLoad = connection.newBulkLoad('#stream_test', (err, rowCount) => {
assert.ok(err);
assert.strictEqual(err.message, 'Canceled.');

assert.isUndefined(rowCount);
});

bulkLoad.addColumn('i', TYPES.Int, { nullable: false });

const rowStream = bulkLoad.getRowStream();
connection.execBulkLoad(bulkLoad);

let rowCount = 0;
const rowSource = Readable.from((async function*() {
while (rowCount < totalRows) {
if (rowCount === 10) {
bulkLoad.cancel();

setTimeout(() => {
assert.strictEqual(connection.state.name, 'LoggedIn');

const request = new Request('select 1', done);

connection.execSql(request);
}, connection.config.options.cancelTimeout + 100);
}

await new Promise((resolve) => {
setTimeout(resolve, 10);
});

yield [rowCount++];
}
})(), { objectMode: true });

pipeline(rowSource, rowStream, (err) => {
assert.ok(err);
assert.strictEqual(err.message, 'Canceled.');
assert.strictEqual(rowCount, 10);
});
});

connection.execSqlBatch(request);
});

it('cancels any bulk load that takes longer than the given timeout', function(done) {
const bulkLoad = connection.newBulkLoad('#tmpTestTable5', { keepNulls: true }, (err, rowCount) => {
assert.instanceOf(err, Error);
Expand Down

0 comments on commit 37f9084

Please sign in to comment.