Skip to content

Conversation

@yigit
Copy link
Collaborator

@yigit yigit commented Jun 19, 2020

This PR fixes an issue where if SourceOfTruth's read or write method fails, we would just crash.

Now SourceOfTruth has two new exception types, WriteException and ReadException where each exception is sent to downstream as a StoreResponse.Error.Exception and stream is kept alive.

If a write fails, reader is simply restarted (because it was previously stopped to prepare for the write).

If a read fails, reader is not restarted until another write attempt happens. This is don't to avoid creating an infinite loop of trying to read from disk.

@yigit yigit requested review from digitalbuddha and eyalgu and removed request for digitalbuddha June 19, 2020 03:43
@yigit
Copy link
Collaborator Author

yigit commented Jun 19, 2020

oh wait sth is wrong i think i need to rebase w/ master

@yigit
Copy link
Collaborator Author

yigit commented Jun 19, 2020

oh i missed the branch name change (:fist_raised: ), rebased over main now, will push.

@yigit yigit force-pushed the yigit/sot-errors-177 branch from 14cdeb0 to da52f4f Compare June 19, 2020 03:47
@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #180 into main will decrease coverage by 1.33%.
The diff coverage is 57.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #180      +/-   ##
============================================
- Coverage     86.01%   84.67%   -1.34%     
- Complexity      230      231       +1     
============================================
  Files            54       54              
  Lines           951      992      +41     
  Branches        149      164      +15     
============================================
+ Hits            818      840      +22     
- Misses           76       85       +9     
- Partials         57       67      +10     
Impacted Files Coverage Δ Complexity Δ
.../dropbox/android/external/store4/impl/RealStore.kt 96.36% <ø> (ø) 16.00 <0.00> (ø)
...m/dropbox/android/external/store4/SourceOfTruth.kt 60.00% <42.85%> (-28.24%) 0.00 <0.00> (ø)
...d/external/store4/impl/SourceOfTruthWithBarrier.kt 86.36% <78.94%> (-7.19%) 7.00 <0.00> (+1.00) ⬇️
.../android/external/store4/impl/RealSourceOfTruth.kt 64.28% <0.00%> (+7.14%) 0.00% <0.00%> (ø%)

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 b860504...e6abbfb. Read the comment docs.

yigit added 5 commits June 19, 2020 07:57
Now source of truth can unblock reader while also letting them
know that an error happened while writing. This forces readers
to first dispatch the error then whatever data they have.

I've also added new public WriteException/ReadException classes
to SourceOfTruth so that it is easy to diagnose these problems
when it hits to the developer's code
@yigit yigit force-pushed the yigit/sot-errors-177 branch from da52f4f to 08f7768 Compare June 19, 2020 15:08
@yigit yigit merged commit 61762f2 into main Jun 27, 2020
rallat pushed a commit that referenced this pull request Aug 13, 2021
* reproduce #177 and fix reader errors

* dispatch write errors to receiver

Now source of truth can unblock reader while also letting them
know that an error happened while writing. This forces readers
to first dispatch the error then whatever data they have.

I've also added new public WriteException/ReadException classes
to SourceOfTruth so that it is easy to diagnose these problems
when it hits to the developer's code
itsandreramon pushed a commit to itsandreramon/Store that referenced this pull request Feb 26, 2025
…MobileNativeFoundation#180)

* reproduce MobileNativeFoundation#177 and fix reader errors

* dispatch write errors to receiver

Now source of truth can unblock reader while also letting them
know that an error happened while writing. This forces readers
to first dispatch the error then whatever data they have.

I've also added new public WriteException/ReadException classes
to SourceOfTruth so that it is easy to diagnose these problems
when it hits to the developer's code
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.

3 participants