Skip to content

Commit f8526df

Browse files
authored
fix(node): Ensure mysql integration works without callback (#9222)
Turns out my "improvement" here actually broke streaming the query response 😬 as in that case you may not provide a callback. This PR fixes this by using `query.on('end')` instead in that case to finish the span. Fixes #9207
1 parent 2875cd9 commit f8526df

File tree

3 files changed

+31
-8
lines changed
  • packages
    • node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback
    • tracing-internal/src/node/integrations

3 files changed

+31
-8
lines changed

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ const connection = mysql.createConnection({
1313
password: 'docker',
1414
});
1515

16+
connection.connect(function (err: unknown) {
17+
if (err) {
18+
return;
19+
}
20+
});
21+
1622
const transaction = Sentry.startTransaction({
1723
op: 'transaction',
1824
name: 'Test Transaction',
@@ -22,10 +28,18 @@ Sentry.configureScope(scope => {
2228
scope.setSpan(transaction);
2329
});
2430

25-
connection.query('SELECT 1 + 1 AS solution');
26-
connection.query('SELECT NOW()', ['1', '2']);
31+
const query = connection.query('SELECT 1 + 1 AS solution');
32+
const query2 = connection.query('SELECT NOW()', ['1', '2']);
33+
34+
query.on('end', () => {
35+
transaction.setTag('result_done', 'yes');
2736

28-
// Wait a bit to ensure the queries completed
29-
setTimeout(() => {
30-
transaction.finish();
31-
}, 500);
37+
query2.on('end', () => {
38+
transaction.setTag('result_done2', 'yes');
39+
40+
// Wait a bit to ensure the queries completed
41+
setTimeout(() => {
42+
transaction.finish();
43+
}, 500);
44+
});
45+
});

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ test('should auto-instrument `mysql` package when using query without callback',
88

99
assertSentryTransaction(envelope[2], {
1010
transaction: 'Test Transaction',
11+
tags: {
12+
result_done: 'yes',
13+
result_done2: 'yes',
14+
},
1115
spans: [
1216
{
1317
description: 'SELECT 1 + 1 AS solution',

packages/tracing-internal/src/node/integrations/mysql.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export class Mysql implements LazyLoadedIntegration<MysqlConnection> {
8484
}
8585

8686
function finishSpan(span: Span | undefined): void {
87-
if (!span) {
87+
if (!span || span.endTimestamp) {
8888
return;
8989
}
9090

@@ -128,9 +128,14 @@ export class Mysql implements LazyLoadedIntegration<MysqlConnection> {
128128
});
129129
}
130130

131-
return orig.call(this, options, values, function () {
131+
// streaming, no callback!
132+
const query = orig.call(this, options, values) as { on: (event: string, callback: () => void) => void };
133+
134+
query.on('end', () => {
132135
finishSpan(span);
133136
});
137+
138+
return query;
134139
};
135140
});
136141
}

0 commit comments

Comments
 (0)