-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add special parsing logic to handle typed array in payload #275
Add special parsing logic to handle typed array in payload #275
Conversation
This pull request has been linked to Shortcut Story #48746: React Native Solana Magic RPC Error: [-32603] Internal error: Expected Buffer. |
β¦eact-native-solana-magic-rpc-error-32603
}, | ||
} as any); | ||
|
||
setTimeout(() => { |
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.
Hopefully we are using https://jestjs.io/docs/timer-mocks here. Can you confirm?
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.
I'm not sure. I was just imitating the other test cases as this is my first unit test in this project.
Based on the context here, this setTimeout
is only used in the unit test, so I don't think we should mock it.
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.
The issue is that unless mocked, each test of this form adds a 100ms to the test run. Not alot but adds up over time. If we mock the setTimeout correctly, then it only adds the time it takes to run the test. Eg, 20 tests would take 10-20ms to run instead of 2000.
For now, lets just leave it but may be worth creating a cooldown ticket for.
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.
Sg
packages/@magic-sdk/react-native/src/react-native-webview-controller.tsx
Show resolved
Hide resolved
|
||
// silently handles exception and return the original copy | ||
} catch (e) { | ||
console.log(e); |
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.
Are we logging this error with enough context that it's obvious to the developer where it originates from? If not, we may want to make use of the MagicSDKWarning
class and log it that way.
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.
I had it removed and left empty. The message wasn't quite useful at all to developers as the code block has the original value returned whenever there's an exception.
If anyone encounters an un-recognizable payload in the future, we may ask the developers to submit their function calls to reproduce the payload.
π PR was released in |
π¦ Pull Request
β Fixed Issues
Fixes #269
π¨ Test instructions
[Describe any additional context required to test the PR/feature/bug fix.]
patch
: Bug Fix?minor
: New Feature?major
: Breaking Change?skip-release
: It's unnecessary to publish this change.