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: Possibly unknown note when creating note. #1797

Merged
merged 2 commits into from
Dec 31, 2019

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Dec 23, 2019

When creating a new note we know that the operation might fail but we
keep on treating it as a success.

In this patch we send a debug message if the note bucket fails to create
our note and we don't continue processing as if it had.

Testing

This is going to be a hard one to test unless you mangle Simperium, so
focus on auditing the change itself and consider the possible side-effects
of it. Make sure that we can still create notes and that those new notes
get synchronized with other devices.

This change is introducing a safety guard where none previously existed.

@dmsnell dmsnell requested a review from a team December 23, 2019 22:21
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

As you said, I can't easily create the conditions for this failure but the code looks fine and a smoke-test didn't surface any issues. Do we want to handle this error in a user-facing way someday too?

@dmsnell
Copy link
Member Author

dmsnell commented Dec 31, 2019

Do we want to handle this error in a user-facing way someday too?

Yes, and I would have here but I didn't know the answer to how we ought to nor know how to quickly estimate what we'd have to do, plus it'd introduce more complexity in the state model I'm trying to undo (the action-creators in action-map), so for now I thought that a debug() call was the least we could do and that it's still better than it is now.

I'd guess that we'd have reports if this happened frequently enough for it to be a problem.

@dmsnell dmsnell force-pushed the fix/possibly-unknown-note branch 2 times, most recently from 69325a1 to 6f18cf7 Compare December 31, 2019 05:21
When creating a new note we know that the operation might fail but we
keep on treating it as a success.

In this patch we send a debug message if the note bucket fails to create
our note and we don't continue processing as if it had.
@dmsnell dmsnell force-pushed the fix/possibly-unknown-note branch from 6f18cf7 to 1ff71d9 Compare December 31, 2019 05:46
@dmsnell dmsnell merged commit 6b0d940 into develop Dec 31, 2019
@dmsnell dmsnell deleted the fix/possibly-unknown-note branch December 31, 2019 06:20
@codebykat codebykat added this to the 1.14 milestone Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants