Skip to content

Conversation

@davimacedo
Copy link
Member

New Pull Request Checklist

  • [ X ] I am not disclosing a vulnerability.
  • [ X ] I am creating this PR in reference to an issue.

Issue Description

Flaky test with mongodb transactions due to transient error.

Related issue: #7180

Approach

It is a weird brute force solution but it is actually the MongoDB recommended way:
https://docs.mongodb.com/manual/core/transactions-in-applications/#core-api

Basically it recommends to retry in an infinite loop every time you get a TransientError. I limited it to 5 times. Before the fix, I testes and it failed about 1 every 100 times the test runs. Now I can run the test 10,000 with no errors.

TODOs before merging

  • Add test cases
  • [ X ] Add entry to changelog
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK
  • ...

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@codecov
Copy link

codecov bot commented Feb 13, 2021

Codecov Report

Merging #7187 (3a2fd82) into master (738ba9f) will decrease coverage by 9.80%.
The diff coverage is 49.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7187      +/-   ##
==========================================
- Coverage   94.04%   84.23%   -9.81%     
==========================================
  Files         172      172              
  Lines       12850    12867      +17     
==========================================
- Hits        12085    10839    -1246     
- Misses        765     2028    +1263     
Impacted Files Coverage Δ
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 90.26% <0.00%> (-3.25%) ⬇️
src/ParseServerRESTController.js 82.08% <56.00%> (-16.28%) ⬇️
src/batch.js 75.86% <56.52%> (-16.45%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 2.42% <0.00%> (-93.44%) ⬇️
src/Adapters/Storage/Postgres/PostgresClient.js 6.66% <0.00%> (-80.00%) ⬇️
src/Controllers/UserController.js 95.27% <0.00%> (-2.37%) ⬇️
src/Controllers/FilesController.js 92.00% <0.00%> (-2.00%) ⬇️
src/Controllers/DatabaseController.js 94.29% <0.00%> (-1.17%) ⬇️
src/Routers/UsersRouter.js 93.75% <0.00%> (-0.63%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 738ba9f...3a2fd82. Read the comment docs.

@davimacedo
Copy link
Member Author

It looks we now have another flaky test because of this. I will check it out before merging.

@davimacedo davimacedo marked this pull request as draft February 13, 2021 00:17
@davimacedo davimacedo marked this pull request as ready for review February 16, 2021 05:52
@davimacedo davimacedo requested a review from mtrezza February 16, 2021 05:52
@davimacedo davimacedo merged commit a430d6f into parse-community:master Feb 18, 2021
@davimacedo davimacedo mentioned this pull request Feb 18, 2021
4 tasks
dplewis pushed a commit that referenced this pull request Feb 21, 2021
* 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
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@mtrezza mtrezza mentioned this pull request Mar 12, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released Released as stable version state:released-beta Released as beta version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants