Add support for compensating writes #5599

papafe · 2023-03-21T11:56:53Z

This PR adds a new error class for compensating writes errors.

☑️ ToDos

📝 Changelog entry
🚦 Tests

kraenhansen

I have a few comments and suggestions.

packages/realm/src/errors.ts

integration-tests/tests/src/tests/sync/flexible.ts

Co-authored-by: Kræn Hansen <[email protected]>

kneth

I have added a couple of suggestions, and they shouldn't block you in any way. Great to see compensating writes coming to a SDK near me 😄

CHANGELOG.md

packages/realm/src/errors.ts

packages/realm/src/Types.ts

Co-authored-by: Kenneth Geisshirt <[email protected]>

* main: Add steps for prerequisites for running MDB aggregation tests. (#5475) Remove dependencies.list (#5598) Removed dependency on bare "mocha-remote" package. Making prebuild-install --verbose Bundle on CI (#5570) Fix example (#5534) Import device-info platform module to React Native bundle (#5594) Chang order of PATH segments, fixing the BaaS test server Download the assisted_agg binary (#5592)

elle-j

Great and thanks 🙏 Added suggestions 👍

CHANGELOG.md

packages/realm/src/Types.ts

packages/realm/src/errors.ts

integration-tests/tests/src/tests/sync/flexible.ts

Co-authored-by: LJ <[email protected]>

packages/realm/src/errors.ts

integration-tests/tests/src/tests/sync/flexible.ts

Co-authored-by: LJ <[email protected]>

nirinchev

This is not a problem with this PR, but it looks like we don't expose the log url in the sync error. We should probably do it as the server logs are quite useful to understand what exactly is failing.

packages/realm/src/errors.ts

integration-tests/tests/src/tests/sync/flexible.ts

nirinchev · 2023-03-22T11:47:53Z

integration-tests/tests/src/tests/sync/flexible.ts

+            realm.create<IDog>(DogSchema.name, { _id: new BSON.ObjectId(), name: "Oldy", age: 6, owner: luigi });
+          });
+
+          await realm?.syncSession?.uploadAllLocalChanges();


Why?all?these?question?marks? 😄

They're fun to use 😁

Looks like it should only be needed after syncSession.

Optional chaining! It replaces this code:

if( realm && realm.syncSession && realm.syncSession.uploadAllLocalChanges){ await realm.syncSession.uploadAllLocalChanges(); }

But maybe we don't even need them?

No, just the second one, I think it's fine :)

I don't think we need either one. If the syncSession is undefined/null, I'd rather that this throws than silently does nothing.

TypeScript will be defensive about this and have a type error if you do not have a check.

Wait, why is Realm.syncSession nullable but Realm.subscriptions not? In any case, this should use ! then to express that we know this is not null.

My guess is that subscriptions can just return an empty array, even if sync isn't enabled. syncSession should return null if sync isn't configured

* main: Prepare for vNext Fixed extraction of version in publish workflow Prepare for 12.0.0-alpha.0 (#5600) Remove redundant MongoDBCollection watch tests. # Conflicts: # CHANGELOG.md

papafe · 2023-03-22T12:54:30Z

@elle-j @kraenhansen @takameyer @kneth
I've just tried to follow all your recommendations so it'll be great if you could give a final (?) look:

The final name for the array property is writes, I hope we can all agree with that in some way 😁
I'm using promiseHandle for the test, instead of that horrible promise with async body
I've changed the type of the primary key from Mixed to PrimaryKey and added an assert method to verify it's of the correct type

packages/realm/src/assert.ts

packages/realm/src/errors.ts

takameyer · 2023-03-22T18:52:10Z

ShipIt

integration-tests/tests/src/tests/sync/flexible.ts

kraenhansen

There's just one thing missing 😬

packages/realm/src/errors.ts

papafe added 3 commits March 17, 2023 16:15

Initial work

aa71dee

Updates

52e20d0

Several fixes

10a9b16

cla-bot bot added the cla: yes label Mar 21, 2023

github-actions bot assigned papafe Mar 21, 2023

papafe added 2 commits March 21, 2023 12:57

Added changelog

ed3d688

Small fixes

132644e

papafe marked this pull request as ready for review March 21, 2023 11:59

papafe requested review from kraenhansen, elle-j, takameyer and kneth March 21, 2023 12:00

kraenhansen reviewed Mar 21, 2023

View reviewed changes

papafe and others added 3 commits March 21, 2023 22:23

Apply suggestions from code review

dacc4de

Co-authored-by: Kræn Hansen <[email protected]>

Apply suggestions from code review

f25bdb0

Co-authored-by: Kræn Hansen <[email protected]>

Various corrections

7ba4d60

kneth approved these changes Mar 22, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

packages/realm/src/errors.ts Show resolved Hide resolved

papafe commented Mar 22, 2023

View reviewed changes

packages/realm/src/Types.ts Outdated Show resolved Hide resolved

papafe and others added 5 commits March 22, 2023 08:50

Apply suggestions from code review [skip-ci]

123f30b

Co-authored-by: Kenneth Geisshirt <[email protected]>

Corrected doc [skip ci]

ebe9a57

Simplification

e962082

Corrected naming [skip ci]

ede3ca1

elle-j approved these changes Mar 22, 2023

View reviewed changes

Apply suggestions from code review [skip ci]

c7eebec

Co-authored-by: LJ <[email protected]>

takameyer reviewed Mar 22, 2023

View reviewed changes

packages/realm/src/errors.ts Outdated Show resolved Hide resolved

takameyer reviewed Mar 22, 2023

View reviewed changes

packages/realm/src/errors.ts Outdated Show resolved Hide resolved

takameyer reviewed Mar 22, 2023

View reviewed changes

integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved

papafe and others added 2 commits March 22, 2023 11:56

Apply suggestions from code review [skip ci]

4da367c

Co-authored-by: LJ <[email protected]>

Various fixes

16adb2e

nirinchev reviewed Mar 22, 2023

View reviewed changes

papafe added 3 commits March 22, 2023 13:44

Small fixes [skip ci]

19dca8e

Removed unused [skip ci]

5f95753

Merge branch 'main' into fp/compensating-writes [skip ci]

0225dde

* main: Prepare for vNext Fixed extraction of version in publish workflow Prepare for 12.0.0-alpha.0 (#5600) Remove redundant MongoDBCollection watch tests. # Conflicts: # CHANGELOG.md

papafe requested review from takameyer, kraenhansen and elle-j March 22, 2023 12:54

kraenhansen reviewed Mar 22, 2023

View reviewed changes

packages/realm/src/assert.ts Show resolved Hide resolved

kraenhansen reviewed Mar 22, 2023

View reviewed changes

packages/realm/src/errors.ts Outdated Show resolved Hide resolved

papafe added 2 commits March 22, 2023 14:06

Final name correction [skip ci]

3151599

Simplifications [skip ci]

3cdd3a6

elle-j approved these changes Mar 22, 2023

View reviewed changes

kraenhansen reviewed Mar 22, 2023

View reviewed changes

integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved

kraenhansen reviewed Mar 22, 2023

View reviewed changes

packages/realm/src/errors.ts Show resolved Hide resolved

takameyer approved these changes Mar 23, 2023

View reviewed changes

Final fixes [skip ci]

0f13c7a

papafe merged commit 9f8b6d8 into main Mar 23, 2023

papafe deleted the fp/compensating-writes branch March 23, 2023 09:14

Omrisnyk mentioned this pull request Nov 28, 2023

[Snyk] Fix for 5 vulnerabilities Omrisnyk/trinity-wallet#75

Open

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for compensating writes #5599

Add support for compensating writes #5599

papafe commented Mar 21, 2023 •

edited

Loading

kraenhansen left a comment •

edited

Loading

kneth left a comment

elle-j left a comment

nirinchev left a comment

nirinchev Mar 22, 2023

papafe Mar 22, 2023

elle-j Mar 22, 2023

takameyer Mar 22, 2023 •

edited

Loading

takameyer Mar 22, 2023

papafe Mar 22, 2023

nirinchev Mar 22, 2023

takameyer Mar 22, 2023

nirinchev Mar 22, 2023 •

edited

Loading

takameyer Mar 23, 2023

papafe commented Mar 22, 2023 •

edited

Loading

takameyer commented Mar 22, 2023

kraenhansen left a comment

Add support for compensating writes #5599

Add support for compensating writes #5599

Conversation

papafe commented Mar 21, 2023 • edited Loading

☑️ ToDos

kraenhansen left a comment • edited Loading

Choose a reason for hiding this comment

kneth left a comment

Choose a reason for hiding this comment

elle-j left a comment

Choose a reason for hiding this comment

nirinchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takameyer Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirinchev Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papafe commented Mar 22, 2023 • edited Loading

takameyer commented Mar 22, 2023

kraenhansen left a comment

Choose a reason for hiding this comment

papafe commented Mar 21, 2023 •

edited

Loading

kraenhansen left a comment •

edited

Loading

takameyer Mar 22, 2023 •

edited

Loading

nirinchev Mar 22, 2023 •

edited

Loading

papafe commented Mar 22, 2023 •

edited

Loading