fix: NitroSQLite not propagating native errors by upgrading Skia to 2.4.18 - #99541
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
|
|
@brunovjk this PR is ready for review! 🙌🏼 |
|
Hey @chrispader, what do you think about this reported Android canvas transparency regression in Skia 2.4.17? Shopify/react-native-skia#3683 It seems to be fixed in 2.4.18. |
|
Ahh thanks for raising that up. Let's bump to 2.4.18 then instead! 🙌🏼 |
|
@brunovjk i've updated the Skia version! |
Reviewer Checklist
Screenshots/Videos |
brunovjk
left a comment
There was a problem hiding this comment.
The diff looks good overall. @chrispader the Mobile-Expensify PR #14083 references the closed App PR #99696. Should we update it to reference this PR (#99541) instead?
@brunovjk done! |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.65-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.4.65-3 🚀
Bundle Size Analysis (Sentry): |


On iOS, Skia 2.4.14's prebuilt C++ libraries do not receive the exception flags required for cross-module C++ exceptions. NitroFetch therefore turns useful URLSession failures into
Unknown std::runtime_error error., which prevents App from classifying network failures. This PR upgrades Skia to 2.4.18, which keeps the exception fix first released in 2.4.17 and avoids the Android canvas transparency regression that 2.4.17 accidentally shipped.@mountiny @mallenexpensify
Explanation of Change
Skia PR #3595 attempted to enable
-fexceptionsand-frttithrough GN'sextra_cflags, but that setting did not apply the flags to C++ sources in the prebuilt Apple libraries. Skia PR #3681 changed the setting toextra_cflags_ccand rebuilt the Apple artifacts. That fix shipped in 2.4.17 and remains in 2.4.18.Skia 2.4.17 also included an accidental test change that made transparent Android canvas backgrounds render black. The maintainer republished the package, and the issue reporter confirmed that 2.4.18 fixes the regression in Shopify/react-native-skia#3683. No App or NitroFetch error handling changes in this PR.
Upstream context:
Fixed Issues
$ #98988
PROPOSAL: #98988
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/14083
Tests
On
main, change thereact-nativeimport inindex.jsto includeAlert, then add the following reproduction block immediately afteradditionalAppSetup():Run the iOS HybridApp development build.
Verify the request rejects with
Unknown std::runtime_error error.while the app uses Skia 2.4.14.Install this branch's dependencies and pods, then rebuild the same iOS target with Skia 2.4.18.
Run the identical request and verify it rejects with
NSURLErrorDomain Code=-1004,Could not connect to the server., and the failing URL instead of the opaque runtime error.Remove the temporary reproduction call and verify no reproduction alert or debug logging remains.
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
The after screenshot was captured with Skia 2.4.17, the first release with the C++ exception fix. The final dependency is 2.4.18 because it keeps that fix and resolves the Android canvas regression in Shopify/react-native-skia#3683.
iOS: Native