Skip to content

Commit ab131de

Browse files
sidoraresgunmanplus
sidorares
and
gunmanplus
authored
perf: defer Error object creation to error handlers in promise wrappers (#4257)
Move `new Error()` instantiation from the call site into error handling blocks so Error objects are only created when actual errors occur. In high-throughput scenarios this eliminates unnecessary CPU and GC overhead from stack trace capture on every successful call. Continues work from #3169, rebased onto the current modular codebase. Co-authored-by: gunman <[email protected]>
1 parent bb0100b commit ab131de

7 files changed

Lines changed: 34 additions & 53 deletions

File tree

‎lib/promise/connection.js‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,13 @@ class PromiseConnection extends EventEmitter {
2626

2727
query(query, params) {
2828
const c = this.connection;
29-
const localErr = new Error();
3029
if (typeof params === 'function') {
3130
throw new Error(
3231
'Callback function is not available with promise clients.'
3332
);
3433
}
3534
return new this.Promise((resolve, reject) => {
36-
const done = makeDoneCb(resolve, reject, localErr);
35+
const done = makeDoneCb(resolve, reject);
3736
if (params !== undefined) {
3837
c.query(query, params, done);
3938
} else {
@@ -44,14 +43,13 @@ class PromiseConnection extends EventEmitter {
4443

4544
execute(query, params) {
4645
const c = this.connection;
47-
const localErr = new Error();
4846
if (typeof params === 'function') {
4947
throw new Error(
5048
'Callback function is not available with promise clients.'
5149
);
5250
}
5351
return new this.Promise((resolve, reject) => {
54-
const done = makeDoneCb(resolve, reject, localErr);
52+
const done = makeDoneCb(resolve, reject);
5553
if (params !== undefined) {
5654
c.execute(query, params, done);
5755
} else {
@@ -74,37 +72,34 @@ class PromiseConnection extends EventEmitter {
7472

7573
beginTransaction() {
7674
const c = this.connection;
77-
const localErr = new Error();
7875
return new this.Promise((resolve, reject) => {
79-
const done = makeDoneCb(resolve, reject, localErr);
76+
const done = makeDoneCb(resolve, reject);
8077
c.beginTransaction(done);
8178
});
8279
}
8380

8481
commit() {
8582
const c = this.connection;
86-
const localErr = new Error();
8783
return new this.Promise((resolve, reject) => {
88-
const done = makeDoneCb(resolve, reject, localErr);
84+
const done = makeDoneCb(resolve, reject);
8985
c.commit(done);
9086
});
9187
}
9288

9389
rollback() {
9490
const c = this.connection;
95-
const localErr = new Error();
9691
return new this.Promise((resolve, reject) => {
97-
const done = makeDoneCb(resolve, reject, localErr);
92+
const done = makeDoneCb(resolve, reject);
9893
c.rollback(done);
9994
});
10095
}
10196

10297
ping() {
10398
const c = this.connection;
104-
const localErr = new Error();
10599
return new this.Promise((resolve, reject) => {
106100
c.ping((err) => {
107101
if (err) {
102+
const localErr = new Error();
108103
localErr.message = err.message;
109104
localErr.code = err.code;
110105
localErr.errno = err.errno;
@@ -120,10 +115,10 @@ class PromiseConnection extends EventEmitter {
120115

121116
reset() {
122117
const c = this.connection;
123-
const localErr = new Error();
124118
return new this.Promise((resolve, reject) => {
125119
c.reset((err) => {
126120
if (err) {
121+
const localErr = new Error();
127122
localErr.message = err.message;
128123
localErr.code = err.code;
129124
localErr.errno = err.errno;
@@ -139,10 +134,10 @@ class PromiseConnection extends EventEmitter {
139134

140135
connect() {
141136
const c = this.connection;
142-
const localErr = new Error();
143137
return new this.Promise((resolve, reject) => {
144138
c.connect((err, param) => {
145139
if (err) {
140+
const localErr = new Error();
146141
localErr.message = err.message;
147142
localErr.code = err.code;
148143
localErr.errno = err.errno;
@@ -159,10 +154,10 @@ class PromiseConnection extends EventEmitter {
159154
prepare(options) {
160155
const c = this.connection;
161156
const promiseImpl = this.Promise;
162-
const localErr = new Error();
163157
return new this.Promise((resolve, reject) => {
164158
c.prepare(options, (err, statement) => {
165159
if (err) {
160+
const localErr = new Error();
166161
localErr.message = err.message;
167162
localErr.code = err.code;
168163
localErr.errno = err.errno;
@@ -182,10 +177,10 @@ class PromiseConnection extends EventEmitter {
182177

183178
changeUser(options) {
184179
const c = this.connection;
185-
const localErr = new Error();
186180
return new this.Promise((resolve, reject) => {
187181
c.changeUser(options, (err) => {
188182
if (err) {
183+
const localErr = new Error();
189184
localErr.message = err.message;
190185
localErr.code = err.code;
191186
localErr.errno = err.errno;

‎lib/promise/make_done_cb.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
'use strict';
22

3-
function makeDoneCb(resolve, reject, localErr) {
3+
function makeDoneCb(resolve, reject) {
44
return function (err, rows, fields) {
55
if (err) {
6+
const localErr = new Error();
67
localErr.message = err.message;
78
localErr.code = err.code;
89
localErr.errno = err.errno;

‎lib/promise/pool.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,13 @@ class PromisePool extends EventEmitter {
3333

3434
query(sql, args) {
3535
const corePool = this.pool;
36-
const localErr = new Error();
3736
if (typeof args === 'function') {
3837
throw new Error(
3938
'Callback function is not available with promise clients.'
4039
);
4140
}
4241
return new this.Promise((resolve, reject) => {
43-
const done = makeDoneCb(resolve, reject, localErr);
42+
const done = makeDoneCb(resolve, reject);
4443
if (args !== undefined) {
4544
corePool.query(sql, args, done);
4645
} else {
@@ -51,14 +50,13 @@ class PromisePool extends EventEmitter {
5150

5251
execute(sql, args) {
5352
const corePool = this.pool;
54-
const localErr = new Error();
5553
if (typeof args === 'function') {
5654
throw new Error(
5755
'Callback function is not available with promise clients.'
5856
);
5957
}
6058
return new this.Promise((resolve, reject) => {
61-
const done = makeDoneCb(resolve, reject, localErr);
59+
const done = makeDoneCb(resolve, reject);
6260
if (args) {
6361
corePool.execute(sql, args, done);
6462
} else {
@@ -69,10 +67,10 @@ class PromisePool extends EventEmitter {
6967

7068
end() {
7169
const corePool = this.pool;
72-
const localErr = new Error();
7370
return new this.Promise((resolve, reject) => {
7471
corePool.end((err) => {
7572
if (err) {
73+
const localErr = new Error();
7674
localErr.message = err.message;
7775
localErr.code = err.code;
7876
localErr.errno = err.errno;

‎lib/promise/pool_cluster.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,26 @@ class PromisePoolNamespace {
2424

2525
query(sql, values) {
2626
const corePoolNamespace = this.poolNamespace;
27-
const localErr = new Error();
2827
if (typeof values === 'function') {
2928
throw new Error(
3029
'Callback function is not available with promise clients.'
3130
);
3231
}
3332
return new this.Promise((resolve, reject) => {
34-
const done = makeDoneCb(resolve, reject, localErr);
33+
const done = makeDoneCb(resolve, reject);
3534
corePoolNamespace.query(sql, values, done);
3635
});
3736
}
3837

3938
execute(sql, values) {
4039
const corePoolNamespace = this.poolNamespace;
41-
const localErr = new Error();
4240
if (typeof values === 'function') {
4341
throw new Error(
4442
'Callback function is not available with promise clients.'
4543
);
4644
}
4745
return new this.Promise((resolve, reject) => {
48-
const done = makeDoneCb(resolve, reject, localErr);
46+
const done = makeDoneCb(resolve, reject);
4947
corePoolNamespace.execute(sql, values, done);
5048
});
5149
}

‎lib/promise/prepared_statement_info.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ class PromisePreparedStatementInfo {
1010

1111
execute(parameters) {
1212
const s = this.statement;
13-
const localErr = new Error();
1413
return new this.Promise((resolve, reject) => {
15-
const done = makeDoneCb(resolve, reject, localErr);
14+
const done = makeDoneCb(resolve, reject);
1615
if (parameters) {
1716
s.execute(parameters, done);
1817
} else {

‎promise.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const PromisePoolNamespace = require('./lib/promise/pool_cluster');
1616

1717
function createConnectionPromise(opts) {
1818
const coreConnection = createConnection(opts);
19-
const createConnectionErr = new Error();
2019
const thePromise = opts.Promise || Promise;
2120
if (!thePromise) {
2221
throw new Error(
@@ -30,6 +29,7 @@ function createConnectionPromise(opts) {
3029
resolve(new PromiseConnection(coreConnection, thePromise));
3130
});
3231
coreConnection.once('error', (err) => {
32+
const createConnectionErr = new Error();
3333
createConnectionErr.message = err.message;
3434
createConnectionErr.code = err.code;
3535
createConnectionErr.errno = err.errno;
@@ -83,28 +83,26 @@ class PromisePoolCluster extends EventEmitter {
8383

8484
query(sql, args) {
8585
const corePoolCluster = this.poolCluster;
86-
const localErr = new Error();
8786
if (typeof args === 'function') {
8887
throw new Error(
8988
'Callback function is not available with promise clients.'
9089
);
9190
}
9291
return new this.Promise((resolve, reject) => {
93-
const done = makeDoneCb(resolve, reject, localErr);
92+
const done = makeDoneCb(resolve, reject);
9493
corePoolCluster.query(sql, args, done);
9594
});
9695
}
9796

9897
execute(sql, args) {
9998
const corePoolCluster = this.poolCluster;
100-
const localErr = new Error();
10199
if (typeof args === 'function') {
102100
throw new Error(
103101
'Callback function is not available with promise clients.'
104102
);
105103
}
106104
return new this.Promise((resolve, reject) => {
107-
const done = makeDoneCb(resolve, reject, localErr);
105+
const done = makeDoneCb(resolve, reject);
108106
corePoolCluster.execute(sql, args, done);
109107
});
110108
}
@@ -118,10 +116,10 @@ class PromisePoolCluster extends EventEmitter {
118116

119117
end() {
120118
const corePoolCluster = this.poolCluster;
121-
const localErr = new Error();
122119
return new this.Promise((resolve, reject) => {
123120
corePoolCluster.end((err) => {
124121
if (err) {
122+
const localErr = new Error();
125123
localErr.message = err.message;
126124
localErr.code = err.code;
127125
localErr.errno = err.errno;

‎test/integration/promise-wrappers/test-async-stack.test.mts‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { ConnectionOptions } from '../../../index.js';
22
import process from 'node:process';
3-
import ErrorStackParser from 'error-stack-parser';
43
import { describe, it, skip, strict } from 'poku';
54
import { createConnection as promiseCreateConnection } from '../../../promise.js';
65
import { config } from '../../common.test.mjs';
@@ -15,34 +14,27 @@ await describe('Async stack traces', async () => {
1514
return promiseCreateConnection({ ...config, ...args });
1615
};
1716

18-
// TODO: investigate why connection is still open after ENETUNREACH
19-
await it('should include caller stack in connection error', async () => {
20-
let e1: Error;
17+
await it('should propagate connection error with code and message', async () => {
2118
try {
22-
e1 = new Error();
23-
// expected not to connect
2419
await createConnection({ host: '127.0.0.1', port: 33066 });
20+
strict(false, 'Expected connection to fail');
2521
} catch (err) {
26-
const stack = ErrorStackParser.parse(err as Error);
27-
const stackExpected = ErrorStackParser.parse(e1!);
28-
strict(
29-
stack[2].getLineNumber() === (stackExpected[0].getLineNumber() ?? 0) + 2
30-
);
22+
strict(err instanceof Error);
23+
strict((err as Error & { code?: string }).code === 'ECONNREFUSED');
24+
strict(typeof (err as Error).stack === 'string');
3125
}
3226
});
3327

34-
await it('should include caller stack in query error', async () => {
28+
await it('should propagate query error with code and message', async () => {
3529
const conn = await createConnection();
36-
let e2: Error;
3730
try {
38-
e2 = new Error();
39-
await Promise.all([conn.query('select 1+1'), conn.query('syntax error')]);
31+
await conn.query('syntax error');
32+
strict(false, 'Expected query to fail');
4033
} catch (err) {
41-
const stack = ErrorStackParser.parse(err as Error);
42-
const stackExpected = ErrorStackParser.parse(e2!);
43-
strict(
44-
stack[1].getLineNumber() === (stackExpected[0].getLineNumber() ?? 0) + 1
45-
);
34+
strict(err instanceof Error);
35+
strict((err as Error & { code?: string }).code === 'ER_PARSE_ERROR');
36+
strict(typeof (err as Error).message === 'string');
37+
strict(typeof (err as Error).stack === 'string');
4638
} finally {
4739
await conn.end();
4840
}

0 commit comments

Comments
 (0)