Skip to content

Commit 7bbad9c

Browse files
mbroadstmbroadst
committed
refactor: make errorLabels a private set of MongoError 
1 parent 5f04660 commit 7bbad9c

File tree

7 files changed

+35
-51
lines changed

7 files changed

+35
-51
lines changed

‎lib/core/error.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const mongoErrorContextSymbol = Symbol('mongoErrorContextSymbol');
4+
const kErrorLabels = Symbol('errorLabels');
45

56
/**
67
* Creates a new MongoError
@@ -21,6 +22,11 @@ class MongoError extends Error {
2122
} else {
2223
super(message.message || message.errmsg || message.$err || 'n/a');
2324
for (var name in message) {
25+
if (name === 'errorLabels') {
26+
this[kErrorLabels] = new Set(message[name]);
27+
continue;
28+
}
29+
2430
this[name] = message[name];
2531
}
2632
}
@@ -49,7 +55,19 @@ class MongoError extends Error {
4955
* @returns {boolean} returns true if the error has the provided error label
5056
*/
5157
hasErrorLabel(label) {
52-
return this.errorLabels && this.errorLabels.indexOf(label) !== -1;
58+
return this[kErrorLabels] && this[kErrorLabels].has(label);
59+
}
60+
61+
addErrorLabel(label) {
62+
if (this[kErrorLabels] == null) {
63+
this[kErrorLabels] = new Set();
64+
}
65+
66+
this[kErrorLabels].add(label);
67+
}
68+
69+
get errorLabels() {
70+
return this[kErrorLabels] ? Array.from(this[kErrorLabels]) : [];
5371
}
5472
}
5573

‎lib/core/sessions.js

+2-14
Original file line numberDiff line numberDiff line change
@@ -374,10 +374,7 @@ function attemptTransaction(session, startTime, fn, options) {
374374
}
375375

376376
if (isMaxTimeMSExpiredError(err)) {
377-
if (err.errorLabels == null) {
378-
err.errorLabels = [];
379-
}
380-
err.errorLabels.push('UnknownTransactionCommitResult');
377+
err.addErrorLabel('UnknownTransactionCommitResult');
381378
}
382379

383380
throw err;
@@ -476,17 +473,8 @@ function endTransaction(session, commandName, callback) {
476473
isRetryableError(e) ||
477474
isMaxTimeMSExpiredError(e))
478475
) {
479-
if (e.errorLabels) {
480-
const idx = e.errorLabels.indexOf('TransientTransactionError');
481-
if (idx !== -1) {
482-
e.errorLabels.splice(idx, 1);
483-
}
484-
} else {
485-
e.errorLabels = [];
486-
}
487-
488476
if (isUnknownTransactionCommitResult(e)) {
489-
e.errorLabels.push('UnknownTransactionCommitResult');
477+
e.addErrorLabel('UnknownTransactionCommitResult');
490478

491479
// per txns spec, must unpin session in this case
492480
session.transaction.unpinServer();

‎lib/core/wireprotocol/command.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,7 @@ function _command(server, ns, cmd, options, callback) {
105105
err instanceof MongoNetworkError &&
106106
!err.hasErrorLabel('TransientTransactionError')
107107
) {
108-
if (err.errorLabels == null) {
109-
err.errorLabels = [];
110-
}
111-
err.errorLabels.push('TransientTransactionError');
108+
err.addErrorLabel('TransientTransactionError');
112109
}
113110

114111
if (

‎test/examples/transactions.js

+4-10
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('examples(transactions):', function() {
4242
console.log('Transaction aborted. Caught exception during transaction.');
4343

4444
// If transient error, retry the whole transaction
45-
if (error.errorLabels && error.errorLabels.indexOf('TransientTransactionError') >= 0) {
45+
if (error.hasErrorLabel('TransientTransactionError')) {
4646
console.log('TransientTransactionError, retrying transaction ...');
4747
await runTransactionWithRetry(txnFunc, client, session);
4848
} else {
@@ -98,10 +98,7 @@ describe('examples(transactions):', function() {
9898
await session.commitTransaction();
9999
console.log('Transaction committed.');
100100
} catch (error) {
101-
if (
102-
error.errorLabels &&
103-
error.errorLabels.indexOf('UnknownTransactionCommitResult') >= 0
104-
) {
101+
if (error.hasErrorLabel('UnknownTransactionCommitResult')) {
105102
console.log('UnknownTransactionCommitResult, retrying commit operation ...');
106103
await commitWithRetry(session);
107104
} else {
@@ -156,10 +153,7 @@ describe('examples(transactions):', function() {
156153
await session.commitTransaction();
157154
console.log('Transaction committed.');
158155
} catch (error) {
159-
if (
160-
error.errorLabels &&
161-
error.errorLabels.indexOf('UnknownTransactionCommitResult') >= 0
162-
) {
156+
if (error.hasErrorLabel('UnknownTransactionCommitResult')) {
163157
console.log('UnknownTransactionCommitResult, retrying commit operation ...');
164158
await commitWithRetry(session);
165159
} else {
@@ -176,7 +170,7 @@ describe('examples(transactions):', function() {
176170
console.log('Transaction aborted. Caught exception during transaction.');
177171

178172
// If transient error, retry the whole transaction
179-
if (error.errorLabels && error.errorLabels.indexOf('TransientTransactionError') >= 0) {
173+
if (error.hasErrorLabel('TransientTransactionError')) {
180174
console.log('TransientTransactionError, retrying transaction ...');
181175
await runTransactionWithRetry(txnFunc, client, session);
182176
} else {

‎test/functional/cmap/connection.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('Connection', function() {
7777
});
7878

7979
it('should support calling back multiple times on exhaust commands', {
80-
metadata: { requires: { mongodb: '>=4.2.0' } },
80+
metadata: { requires: { mongodb: '>=4.2.0' }, topology: ['single'] },
8181
test: function(done) {
8282
const ns = `${this.configuration.db}.$cmd`;
8383
const connectOptions = Object.assign(

‎test/functional/operation_promises_example.test.js

+5-11
Original file line numberDiff line numberDiff line change
@@ -7126,7 +7126,7 @@ describe('Operation (Promises)', function() {
71267126
return client.withSession(session => {
71277127
function commit() {
71287128
return session.commitTransaction().catch(e => {
7129-
if (e.errorLabels && e.errorLabels.indexOf('UnknownTransactionCommitResult') < 0) {
7129+
if (e.hasErrorLabel('UnknownTransactionCommitResult')) {
71307130
// LINE console.log('Transaction aborted. Caught exception during transaction.');
71317131
return commit();
71327132
}
@@ -7184,7 +7184,7 @@ describe('Operation (Promises)', function() {
71847184
// LINE console.log('Transaction aborted. Caught exception during transaction.');
71857185

71867186
// If transient error, retry the whole transaction
7187-
if (error.errorLabels && error.errorLabels.indexOf('TransientTransactionError') < 0) {
7187+
if (error.hasErrorLabel('TransientTransactionError')) {
71887188
// LINE console.log('TransientTransactionError, retrying transaction ...');
71897189
return runTransactionWithRetry(txnFunc, client, session);
71907190
}
@@ -7246,10 +7246,7 @@ describe('Operation (Promises)', function() {
72467246
.commitTransaction()
72477247
// LINE .then(() => console.log('Transaction committed.'))
72487248
.catch(error => {
7249-
if (
7250-
error.errorLabels &&
7251-
error.errorLabels.indexOf('UnknownTransactionCommitResult') < 0
7252-
) {
7249+
if (error.hasErrorLabel('UnknownTransactionCommitResult')) {
72537250
// LINE console.log('UnknownTransactionCommitResult, retrying commit operation ...');
72547251
return commitWithRetry(session);
72557252
}
@@ -7310,10 +7307,7 @@ describe('Operation (Promises)', function() {
73107307
.commitTransaction()
73117308
// LINE .then(() => console.log('Transaction committed.'))
73127309
.catch(error => {
7313-
if (
7314-
error.errorLabels &&
7315-
error.errorLabels.indexOf('UnknownTransactionCommitResult') < 0
7316-
) {
7310+
if (error.hasErrorLabel('UnknownTransactionCommitResult')) {
73177311
// LINE console.log('UnknownTransactionCommitResult, retrying commit operation ...');
73187312
return commitWithRetry(session);
73197313
}
@@ -7328,7 +7322,7 @@ describe('Operation (Promises)', function() {
73287322
// LINE console.log('Transaction aborted. Caught exception during transaction.');
73297323

73307324
// If transient error, retry the whole transaction
7331-
if (error.errorLabels && error.errorLabels.indexOf('TransientTransactionError') < 0) {
7325+
if (error.hasErrorLabel('TransientTransactionError')) {
73327326
// LINE console.log('TransientTransactionError, retrying transaction ...');
73337327
return runTransactionWithRetry(txnFunc, client, session);
73347328
}

‎test/functional/transactions.test.js

+3-10
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ describe('Transactions', function() {
3535
'count',
3636
// This test needs there to be multiple mongoses
3737
'increment txnNumber',
38-
// There is something wrong with the distinct command in the runner:
39-
// it is not failing properly
40-
'add transient label to connection errors',
4138
// Skipping this until SPEC-1320 is resolved
4239
'remain pinned after non-transient error on commit'
4340
];
@@ -156,10 +153,8 @@ describe('Transactions', function() {
156153
expect(session.inTransaction()).to.be.true;
157154

158155
coll.insertOne({ b: 2 }, { session }, err => {
159-
expect(err)
160-
.to.exist.and.to.be.an.instanceof(MongoNetworkError)
161-
.and.to.have.a.property('errorLabels')
162-
.that.includes('TransientTransactionError');
156+
expect(err).to.exist.and.to.be.an.instanceof(MongoNetworkError);
157+
expect(err.hasErrorLabel('TransientTransactionError')).to.be.true;
163158

164159
session.abortTransaction(() => session.endSession(() => client.close(done)));
165160
});
@@ -191,9 +186,7 @@ describe('Transactions', function() {
191186
err => {
192187
expect(err).to.not.exist;
193188
coll.insertOne({ a: 1 }, err => {
194-
expect(err)
195-
.to.exist.and.to.be.an.instanceOf(MongoNetworkError)
196-
.and.to.not.have.a.property('errorLabels');
189+
expect(err).to.exist.and.to.be.an.instanceOf(MongoNetworkError);
197190
client.close(done);
198191
});
199192
}

0 commit comments

Comments
 (0)