-
-
Notifications
You must be signed in to change notification settings - Fork 609
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 issue in sync when crypto is not supported by client #2715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your pull request!
I believe it would be important to add a smoke test to ensure that the js-sdk can process a sync when the crypto layer is missing
If I look a little further at this, I can see that the type definition here fails to mention that the crypto
property can be undefined
(see
Line 390 in e1edd84
public crypto: Crypto; // XXX: Intended private, used in code. |
/me will handle this and write a test, thanks for review @gsouquet and thanks @stas-demydiuk for the report & fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍 Good luck with the strict null checks
Co-authored-by: Michael Telatynski <[email protected]> (cherry picked from commit 9bb5afe)
When the Matrix client is initialized without crypto support it now breaks in the sync as starting from matrix-js-sdk 19.7.0 the additional code was added that directly access the client's crypto feature.
This PR is aimed to fix the issue by introducing the additional check that crypto is actually available before accessing it
Signed-off-by: Stanislav Demydiuk [email protected]
Checklist
Here's what your changelog entry will look like:
🐛 Bug Fixes