Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky test with transactions #7187

Merged
merged 6 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
__BREAKING CHANGES:__
- NEW: Added file upload restriction. File upload is now only allowed for authenticated users by default for improved security. To allow file upload also for Anonymous Users or Public, set the `fileUpload` parameter in the [Parse Server Options](https://parseplatform.org/parse-server/api/master/ParseServerOptions.html). [#7071](https://github.com/parse-community/parse-server/pull/7071). Thanks to [dblythy](https://github.com/dblythy), [Manuel Trezza](https://github.com/mtrezza).
___
- 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).
- UPGRADE: 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).
- NEW (EXPERIMENTAL): Added new page router with placeholder rendering and localization of custom and feature pages such as password reset and email verification. **Caution, this is an experimental feature that may not be appropriate for production.** [#6891](https://github.com/parse-community/parse-server/issues/6891). Thanks to [Manuel Trezza](https://github.com/mtrezza).
- NEW: Added convenience method `Parse.Cloud.sendEmail(...)` to send email via email adapter in Cloud Code. [#7089](https://github.com/parse-community/parse-server/pull/7089). Thanks to [dblythy](https://github.com/dblythy)
Expand Down
7 changes: 4 additions & 3 deletions spec/ParseServerRESTController.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('ParseServerRESTController', () => {
.then(() => {
spyOn(databaseAdapter, 'createObject').and.callThrough();

RESTController.request('POST', 'batch', {
return RESTController.request('POST', 'batch', {
requests: [
{
method: 'POST',
Expand All @@ -218,7 +218,7 @@ describe('ParseServerRESTController', () => {
expect(response[1].success.objectId).toBeDefined();
expect(response[1].success.createdAt).toBeDefined();
const query = new Parse.Query('MyObject');
query.find().then(results => {
return query.find().then(results => {
expect(databaseAdapter.createObject.calls.count()).toBe(2);
expect(databaseAdapter.createObject.calls.argsFor(0)[3]).toBe(
databaseAdapter.createObject.calls.argsFor(1)[3]
Expand All @@ -230,7 +230,8 @@ describe('ParseServerRESTController', () => {
done();
});
});
});
})
.catch(done.fail);
});

it('should not save anything when one operation fails in a transaction', done => {
Expand Down
17 changes: 14 additions & 3 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -1046,9 +1046,20 @@ export class MongoStorageAdapter implements StorageAdapter {
}

commitTransactionalSession(transactionalSection: any): Promise<void> {
return transactionalSection.commitTransaction().then(() => {
transactionalSection.endSession();
});
const commit = retries => {
return transactionalSection
.commitTransaction()
.catch(error => {
if (error && error.hasErrorLabel('TransientTransactionError') && retries > 0) {
return commit(retries - 1);
}
throw error;
})
.then(() => {
transactionalSection.endSession();
});
};
return commit(5);
}

abortTransactionalSession(transactionalSection: any): Promise<void> {
Expand Down
86 changes: 51 additions & 35 deletions src/ParseServerRESTController.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,44 +48,60 @@ function ParseServerRESTController(applicationId, router) {
}

if (path === '/batch') {
let initialPromise = Promise.resolve();
if (data.transaction === true) {
initialPromise = config.database.createTransactionalSession();
}
return initialPromise.then(() => {
const promises = data.requests.map(request => {
return handleRequest(request.method, request.path, request.body, options, config).then(
response => {
if (options.returnStatus) {
const status = response._status;
delete response._status;
return { success: response, _status: status };
const batch = transactionRetries => {
let initialPromise = Promise.resolve();
if (data.transaction === true) {
initialPromise = config.database.createTransactionalSession();
}
return initialPromise.then(() => {
const promises = data.requests.map(request => {
return handleRequest(request.method, request.path, request.body, options, config).then(
response => {
if (options.returnStatus) {
const status = response._status;
delete response._status;
return { success: response, _status: status };
}
return { success: response };
},
error => {
return {
error: { code: error.code, error: error.message },
};
}
return { success: response };
},
error => {
return {
error: { code: error.code, error: error.message },
};
}
);
});
return Promise.all(promises).then(result => {
if (data.transaction === true) {
if (result.find(resultItem => typeof resultItem.error === 'object')) {
return config.database.abortTransactionalSession().then(() => {
return Promise.reject(result);
});
} else {
return config.database.commitTransactionalSession().then(() => {
);
});
return Promise.all(promises)
.then(result => {
if (data.transaction === true) {
if (result.find(resultItem => typeof resultItem.error === 'object')) {
return config.database.abortTransactionalSession().then(() => {
return Promise.reject(result);
});
} else {
return config.database.commitTransactionalSession().then(() => {
return result;
});
}
} else {
return result;
});
}
} else {
return result;
}
}
})
.catch(error => {
if (
error &&
error.find(
errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251
) &&
transactionRetries > 0
) {
return batch(transactionRetries - 1);
}
throw error;
});
});
});
};
return batch(5);
}

let query;
Expand Down
97 changes: 57 additions & 40 deletions src/batch.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,49 +83,66 @@ function handleBatch(router, req) {
req.config.publicServerURL
);

let initialPromise = Promise.resolve();
if (req.body.transaction === true) {
initialPromise = req.config.database.createTransactionalSession();
}

return initialPromise.then(() => {
const promises = req.body.requests.map(restRequest => {
const routablePath = makeRoutablePath(restRequest.path);

// Construct a request that we can send to a handler
const request = {
body: restRequest.body,
config: req.config,
auth: req.auth,
info: req.info,
};

return router.tryRouteRequest(restRequest.method, routablePath, request).then(
response => {
return { success: response.response };
},
error => {
return { error: { code: error.code, error: error.message } };
}
);
});
const batch = transactionRetries => {
let initialPromise = Promise.resolve();
if (req.body.transaction === true) {
initialPromise = req.config.database.createTransactionalSession();
}

return Promise.all(promises).then(results => {
if (req.body.transaction === true) {
if (results.find(result => typeof result.error === 'object')) {
return req.config.database.abortTransactionalSession().then(() => {
return Promise.reject({ response: results });
});
} else {
return req.config.database.commitTransactionalSession().then(() => {
return initialPromise.then(() => {
const promises = req.body.requests.map(restRequest => {
const routablePath = makeRoutablePath(restRequest.path);

// Construct a request that we can send to a handler
const request = {
body: restRequest.body,
config: req.config,
auth: req.auth,
info: req.info,
};

return router.tryRouteRequest(restRequest.method, routablePath, request).then(
response => {
return { success: response.response };
},
error => {
return { error: { code: error.code, error: error.message } };
}
);
});

return Promise.all(promises)
.then(results => {
if (req.body.transaction === true) {
if (results.find(result => typeof result.error === 'object')) {
return req.config.database.abortTransactionalSession().then(() => {
return Promise.reject({ response: results });
});
} else {
return req.config.database.commitTransactionalSession().then(() => {
return { response: results };
});
}
} else {
return { response: results };
});
}
} else {
return { response: results };
}
}
})
.catch(error => {
if (
error &&
error.response &&
error.response.find(
errorItem => typeof errorItem.error === 'object' && errorItem.error.code === 251
) &&
transactionRetries > 0
) {
return batch(transactionRetries - 1);
}
throw error;
});
});
});
};
return batch(5);
}

module.exports = {
Expand Down