Incorrect MessageOptions for RecordsRead requests in sync manager.
#297
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LiranCohen
requested review from
frankhinek,
csuwildcat and
mistermoe
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
TBDocs Report ✅ No errors or warnings @web5/api
Updated @ 2023-11-18T14:59:36.687Z - Commit: |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #297 +/- ##
==========================================
+ Coverage 90.41% 90.70% +0.28%
==========================================
Files 74 74
Lines 14068 14075 +7
Branches 1379 1384 +5
==========================================
+ Hits 12720 12767 +47
+ Misses 1322 1282 -40
Partials 26 26
|
frankhinek
force-pushed
the
lirancohen/sync-error-get-dwn-message
branch
from
2ecbc7b to
aae47db
Compare
frankhinek
approved these changes
Nov 18, 2023
csuwildcat
approved these changes
Nov 18, 2023
finn-block
pushed a commit
that referenced
this pull request
Mar 19, 2024
…297) * add filter property for RecordsRead within sync manager * Fix response type for RecordsRead messages and improve type safety --------- Co-authored-by: Frank Hinek <[email protected]>
finn-block
pushed a commit
that referenced
this pull request
Mar 19, 2024
…297) * add filter property for RecordsRead within sync manager * Fix response type for RecordsRead messages and improve type safety --------- Co-authored-by: Frank Hinek <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the sync-manager issued a
RecordsReadit was not updated to use the filter property that was added.I'll create a separate issue for
dwn-sdk-jsto handle this error and return a proper response.This addresses #294
The reason this would happen in those scenarios, after updating the initial
RecordsWritemessage is retained, but the data is deleted (including theencodedData).This error would happen during a
pullwhere the client is pulling down messages associated with a record that's been updated, when they reach the initial write, doesn't find it locally, and then does aRecordsReadto fetch it, it failed by not providing a filter.It could also happen in other cases when using the
sendRequestmethod from DwnManager.Both of the cases are handled by the tests I added, but I'd like to add some more comprehensive
scenariotesting. Although I think some of that would be better suited for theapipackage as that's how those scenarios would be called.