-
Notifications
You must be signed in to change notification settings - Fork 21
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: Refactors createFromKeyBundle swift function to guard string conv #70
Fix: Refactors createFromKeyBundle swift function to guard string conv #70
Conversation
Thanks so much for doing this!! I think it might be worth writing a test for this error maybe something like the below in this file https://github.com/xmtp/xmtp-react-native/blob/main/example/src/tests.ts . I ran this test below on main and saw the crash and also on your branch and saw it pass so can confirm this fixes it. import { randomBytes } from "crypto";
test("createFromKeyBundle throws error for non string value", async () => {
try {
const bytes = randomBytes(32)
await XMTP.Client.createFromKeyBundle(JSON.stringify(bytes), "local");
} catch (e) {
return true;
}
return false;
}); Unfortunately our CI times out 75% of the time so we have to just run the unit tests locally in iOS and Android before merging. But hopefully I can get a fix for that in soon. But just giving you heads up that CI may be red even if its not. |
android/src/main/java/expo/modules/xmtpreactnativesdk/XMTPModule.kt
Outdated
Show resolved
Hide resolved
… returning an empty string
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.
🎉 This PR is included in version 1.6.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This fixes the issue #66 when the app crashes while dealing with non String types on
createFromKeyBundle
.Taken Actions:
invalidKeyBundle
.How to test it?
createFromKeyBundle
func.