Skip to content

Commit a430d6f

Browse files
davimacedodavimacedo
authored
Fix flaky test with transactions (#7187)
* Fix flaky test with transactions * Add CHANGELOG entry * Fix the other transactions related tests that became flaky because now Parse Server tries to submit the transaction multilpe times in the case of TransientError * Remove fit from tests
1 parent 9a9fc5f commit a430d6f

File tree

6 files changed

+161
-103
lines changed

6 files changed

+161
-103
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ ___
2020
- NEW: LiveQuery support for $and, $nor, $containedBy, $geoWithin, $geoIntersects queries [#7113](https://github.com/parse-community/parse-server/pull/7113). Thanks to [dplewis](https://github.com/dplewis)
2121
- NEW: Supporting patterns in LiveQuery server's config parameter `classNames` [#7131](https://github.com/parse-community/parse-server/pull/7131). Thanks to [Nes-si](https://github.com/Nes-si)
2222
- NEW: `requireAnyUserRoles` and `requireAllUserRoles` for Parse Cloud validator. [#7097](https://github.com/parse-community/parse-server/pull/7097). Thanks to [dblythy](https://github.com/dblythy)
23+
- IMPROVE: Retry transactions on MongoDB when it fails due to transient error [#7187](https://github.com/parse-community/parse-server/pull/7187). Thanks to [Antonio Davi Macedo Coelho de Castro](https://github.com/davimacedo).
2324
- IMPROVE: Bump tests to use Mongo 4.4.4 [#7184](https://github.com/parse-community/parse-server/pull/7184). Thanks to [Antonio Davi Macedo Coelho de Castro](https://github.com/davimacedo).
2425
- IMPROVE: Added new account lockout policy option `accountLockout.unlockOnPasswordReset` to automatically unlock account on password reset. [#7146](https://github.com/parse-community/parse-server/pull/7146). Thanks to [Manuel Trezza](https://github.com/mtrezza).
2526
- IMPROVE: Parse Server is from now on continuously tested against all recent MongoDB versions that have not reached their end-of-life support date. Added MongoDB compatibility table to Parse Server docs. [7161](https://github.com/parse-community/parse-server/pull/7161). Thanks to [Manuel Trezza](https://github.com/mtrezza).

‎spec/ParseServerRESTController.spec.js

+21-14
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ describe('ParseServerRESTController', () => {
197197
.then(() => {
198198
spyOn(databaseAdapter, 'createObject').and.callThrough();
199199

200-
RESTController.request('POST', 'batch', {
200+
return RESTController.request('POST', 'batch', {
201201
requests: [
202202
{
203203
method: 'POST',
@@ -218,19 +218,23 @@ describe('ParseServerRESTController', () => {
218218
expect(response[1].success.objectId).toBeDefined();
219219
expect(response[1].success.createdAt).toBeDefined();
220220
const query = new Parse.Query('MyObject');
221-
query.find().then(results => {
222-
expect(databaseAdapter.createObject.calls.count()).toBe(2);
223-
expect(databaseAdapter.createObject.calls.argsFor(0)[3]).toBe(
224-
databaseAdapter.createObject.calls.argsFor(1)[3]
225-
);
221+
return query.find().then(results => {
222+
expect(databaseAdapter.createObject.calls.count() % 2).toBe(0);
223+
expect(databaseAdapter.createObject.calls.count() > 0).toEqual(true);
224+
for (let i = 0; i + 1 < databaseAdapter.createObject.calls.length; i = i + 2) {
225+
expect(databaseAdapter.createObject.calls.argsFor(i)[3]).toBe(
226+
databaseAdapter.createObject.calls.argsFor(i + 1)[3]
227+
);
228+
}
226229
expect(results.map(result => result.get('key')).sort()).toEqual([
227230
'value1',
228231
'value2',
229232
]);
230233
done();
231234
});
232235
});
233-
});
236+
})
237+
.catch(done.fail);
234238
});
235239

236240
it('should not save anything when one operation fails in a transaction', done => {
@@ -513,18 +517,18 @@ describe('ParseServerRESTController', () => {
513517
const results3 = await query3.find();
514518
expect(results3.map(result => result.get('key')).sort()).toEqual(['value1', 'value2']);
515519

516-
expect(databaseAdapter.createObject.calls.count()).toBe(13);
520+
expect(databaseAdapter.createObject.calls.count() >= 13).toEqual(true);
517521
let transactionalSession;
518522
let transactionalSession2;
519523
let myObjectDBCalls = 0;
520524
let myObject2DBCalls = 0;
521525
let myObject3DBCalls = 0;
522-
for (let i = 0; i < 13; i++) {
526+
for (let i = 0; i < databaseAdapter.createObject.calls.count(); i++) {
523527
const args = databaseAdapter.createObject.calls.argsFor(i);
524528
switch (args[0]) {
525529
case 'MyObject':
526530
myObjectDBCalls++;
527-
if (!transactionalSession) {
531+
if (!transactionalSession || (myObjectDBCalls - 1) % 2 === 0) {
528532
transactionalSession = args[3];
529533
} else {
530534
expect(transactionalSession).toBe(args[3]);
@@ -535,7 +539,7 @@ describe('ParseServerRESTController', () => {
535539
break;
536540
case 'MyObject2':
537541
myObject2DBCalls++;
538-
if (!transactionalSession2) {
542+
if (!transactionalSession2 || (myObject2DBCalls - 1) % 9 === 0) {
539543
transactionalSession2 = args[3];
540544
} else {
541545
expect(transactionalSession2).toBe(args[3]);
@@ -550,9 +554,12 @@ describe('ParseServerRESTController', () => {
550554
break;
551555
}
552556
}
553-
expect(myObjectDBCalls).toEqual(2);
554-
expect(myObject2DBCalls).toEqual(9);
555-
expect(myObject3DBCalls).toEqual(2);
557+
expect(myObjectDBCalls % 2).toEqual(0);
558+
expect(myObjectDBCalls > 0).toEqual(true);
559+
expect(myObject2DBCalls % 9).toEqual(0);
560+
expect(myObject2DBCalls > 0).toEqual(true);
561+
expect(myObject3DBCalls % 2).toEqual(0);
562+
expect(myObject3DBCalls > 0).toEqual(true);
556563
});
557564
});
558565
}

‎spec/batch.spec.js

+17-11
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,13 @@ describe('batch', () => {
228228
expect(response.data[1].success.createdAt).toBeDefined();
229229
const query = new Parse.Query('MyObject');
230230
query.find().then(results => {
231-
expect(databaseAdapter.createObject.calls.count()).toBe(2);
232-
expect(databaseAdapter.createObject.calls.argsFor(0)[3]).toBe(
233-
databaseAdapter.createObject.calls.argsFor(1)[3]
234-
);
231+
expect(databaseAdapter.createObject.calls.count() % 2).toBe(0);
232+
expect(databaseAdapter.createObject.calls.count() > 0).toEqual(true);
233+
for (let i = 0; i + 1 < databaseAdapter.createObject.calls.length; i = i + 2) {
234+
expect(databaseAdapter.createObject.calls.argsFor(i)[3]).toBe(
235+
databaseAdapter.createObject.calls.argsFor(i + 1)[3]
236+
);
237+
}
235238
expect(results.map(result => result.get('key')).sort()).toEqual([
236239
'value1',
237240
'value2',
@@ -542,18 +545,18 @@ describe('batch', () => {
542545
const results3 = await query3.find();
543546
expect(results3.map(result => result.get('key')).sort()).toEqual(['value1', 'value2']);
544547

545-
expect(databaseAdapter.createObject.calls.count()).toBe(13);
548+
expect(databaseAdapter.createObject.calls.count() >= 13).toEqual(true);
546549
let transactionalSession;
547550
let transactionalSession2;
548551
let myObjectDBCalls = 0;
549552
let myObject2DBCalls = 0;
550553
let myObject3DBCalls = 0;
551-
for (let i = 0; i < 13; i++) {
554+
for (let i = 0; i < databaseAdapter.createObject.calls.count(); i++) {
552555
const args = databaseAdapter.createObject.calls.argsFor(i);
553556
switch (args[0]) {
554557
case 'MyObject':
555558
myObjectDBCalls++;
556-
if (!transactionalSession) {
559+
if (!transactionalSession || (myObjectDBCalls - 1) % 2 === 0) {
557560
transactionalSession = args[3];
558561
} else {
559562
expect(transactionalSession).toBe(args[3]);
@@ -564,7 +567,7 @@ describe('batch', () => {
564567
break;
565568
case 'MyObject2':
566569
myObject2DBCalls++;
567-
if (!transactionalSession2) {
570+
if (!transactionalSession2 || (myObject2DBCalls - 1) % 9 === 0) {
568571
transactionalSession2 = args[3];
569572
} else {
570573
expect(transactionalSession2).toBe(args[3]);
@@ -579,9 +582,12 @@ describe('batch', () => {
579582
break;
580583
}
581584
}
582-
expect(myObjectDBCalls).toEqual(2);
583-
expect(myObject2DBCalls).toEqual(9);
584-
expect(myObject3DBCalls).toEqual(2);
585+
expect(myObjectDBCalls % 2).toEqual(0);
586+
expect(myObjectDBCalls > 0).toEqual(true);
587+
expect(myObject2DBCalls % 9).toEqual(0);
588+
expect(myObject2DBCalls > 0).toEqual(true);
589+
expect(myObject3DBCalls % 2).toEqual(0);
590+
expect(myObject3DBCalls > 0).toEqual(true);
585591
});
586592
});
587593
}

‎src/Adapters/Storage/Mongo/MongoStorageAdapter.js

+14-3
Original file line numberDiff line numberDiff line change
@@ -1046,9 +1046,20 @@ export class MongoStorageAdapter implements StorageAdapter {
10461046
}
10471047

10481048
commitTransactionalSession(transactionalSection: any): Promise<void> {
1049-
return transactionalSection.commitTransaction().then(() => {
1050-
transactionalSection.endSession();
1051-
});
1049+
const commit = retries => {
1050+
return transactionalSection
1051+
.commitTransaction()
1052+
.catch(error => {
1053+
if (error && error.hasErrorLabel('TransientTransactionError') && retries > 0) {
1054+
return commit(retries - 1);
1055+
}
1056+
throw error;
1057+
})
1058+
.then(() => {
1059+
transactionalSection.endSession();
1060+
});
1061+
};
1062+
return commit(5);
10521063
}
10531064

10541065
abortTransactionalSession(transactionalSection: any): Promise<void> {

‎src/ParseServerRESTController.js

+51-35
Original file line numberDiff line numberDiff line change
@@ -48,44 +48,60 @@ function ParseServerRESTController(applicationId, router) {
4848
}
4949

5050
if (path === '/batch') {
51-
let initialPromise = Promise.resolve();
52-
if (data.transaction === true) {
53-
initialPromise = config.database.createTransactionalSession();
54-
}
55-
return initialPromise.then(() => {
56-
const promises = data.requests.map(request => {
57-
return handleRequest(request.method, request.path, request.body, options, config).then(
58-
response => {
59-
if (options.returnStatus) {
60-
const status = response._status;
61-
delete response._status;
62-
return { success: response, _status: status };
51+
const batch = transactionRetries => {
52+
let initialPromise = Promise.resolve();
53+
if (data.transaction === true) {
54+
initialPromise = config.database.createTransactionalSession();
55+
}
56+
return initialPromise.then(() => {
57+
const promises = data.requests.map(request => {
58+
return handleRequest(request.method, request.path, request.body, options, config).then(
59+
response => {
60+
if (options.returnStatus) {
61+
const status = response._status;
62+
delete response._status;
63+
return { success: response, _status: status };
64+
}
65+
return { success: response };
66+
},
67+
error => {
68+
return {
69+
error: { code: error.code, error: error.message },
70+
};
6371
}
64-
return { success: response };
65-
},
66-
error => {
67-
return {
68-
error: { code: error.code, error: error.message },
69-
};
70-
}
71-
);
72-
});
73-
return Promise.all(promises).then(result => {
74-
if (data.transaction === true) {
75-
if (result.find(resultItem => typeof resultItem.error === 'object')) {
76-
return config.database.abortTransactionalSession().then(() => {
77-
return Promise.reject(result);
78-
});
79-
} else {
80-
return config.database.commitTransactionalSession().then(() => {
72+
);
73+
});
74+
return Promise.all(promises)
75+
.then(result => {
76+
if (data.transaction === true) {
77+
if (result.find(resultItem => typeof resultItem.error === 'object')) {
78+
return config.database.abortTransactionalSession().then(() => {
79+
return Promise.reject(result);
80+
});
81+
} else {
82+
return config.database.commitTransactionalSession().then(() => {
83+
return result;
84+
});
85+
}
86+
} else {
8187
return result;
82-
});
83-
}
84-
} else {
85-
return result;
86-
}
88+
}
89+
})
90+
.catch(error => {
91+
if (
92+
error &&
93+
error.find(
94+
errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251
95+
) &&
96+
transactionRetries > 0
97+
) {
98+
return batch(transactionRetries - 1);
99+
}
100+
throw error;
101+
});
87102
});
88-
});
103+
};
104+
return batch(5);
89105
}
90106

91107
let query;

‎src/batch.js

+57-40
Original file line numberDiff line numberDiff line change
@@ -83,49 +83,66 @@ function handleBatch(router, req) {
8383
req.config.publicServerURL
8484
);
8585

86-
let initialPromise = Promise.resolve();
87-
if (req.body.transaction === true) {
88-
initialPromise = req.config.database.createTransactionalSession();
89-
}
90-
91-
return initialPromise.then(() => {
92-
const promises = req.body.requests.map(restRequest => {
93-
const routablePath = makeRoutablePath(restRequest.path);
94-
95-
// Construct a request that we can send to a handler
96-
const request = {
97-
body: restRequest.body,
98-
config: req.config,
99-
auth: req.auth,
100-
info: req.info,
101-
};
102-
103-
return router.tryRouteRequest(restRequest.method, routablePath, request).then(
104-
response => {
105-
return { success: response.response };
106-
},
107-
error => {
108-
return { error: { code: error.code, error: error.message } };
109-
}
110-
);
111-
});
86+
const batch = transactionRetries => {
87+
let initialPromise = Promise.resolve();
88+
if (req.body.transaction === true) {
89+
initialPromise = req.config.database.createTransactionalSession();
90+
}
11291

113-
return Promise.all(promises).then(results => {
114-
if (req.body.transaction === true) {
115-
if (results.find(result => typeof result.error === 'object')) {
116-
return req.config.database.abortTransactionalSession().then(() => {
117-
return Promise.reject({ response: results });
118-
});
119-
} else {
120-
return req.config.database.commitTransactionalSession().then(() => {
92+
return initialPromise.then(() => {
93+
const promises = req.body.requests.map(restRequest => {
94+
const routablePath = makeRoutablePath(restRequest.path);
95+
96+
// Construct a request that we can send to a handler
97+
const request = {
98+
body: restRequest.body,
99+
config: req.config,
100+
auth: req.auth,
101+
info: req.info,
102+
};
103+
104+
return router.tryRouteRequest(restRequest.method, routablePath, request).then(
105+
response => {
106+
return { success: response.response };
107+
},
108+
error => {
109+
return { error: { code: error.code, error: error.message } };
110+
}
111+
);
112+
});
113+
114+
return Promise.all(promises)
115+
.then(results => {
116+
if (req.body.transaction === true) {
117+
if (results.find(result => typeof result.error === 'object')) {
118+
return req.config.database.abortTransactionalSession().then(() => {
119+
return Promise.reject({ response: results });
120+
});
121+
} else {
122+
return req.config.database.commitTransactionalSession().then(() => {
123+
return { response: results };
124+
});
125+
}
126+
} else {
121127
return { response: results };
122-
});
123-
}
124-
} else {
125-
return { response: results };
126-
}
128+
}
129+
})
130+
.catch(error => {
131+
if (
132+
error &&
133+
error.response &&
134+
error.response.find(
135+
errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251
136+
) &&
137+
transactionRetries > 0
138+
) {
139+
return batch(transactionRetries - 1);
140+
}
141+
throw error;
142+
});
127143
});
128-
});
144+
};
145+
return batch(5);
129146
}
130147

131148
module.exports = {

0 commit comments

Comments
 (0)