Skip to content

Conversation

@mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Feb 25, 2025

Description

This PR adds a release mode check to our CI to reproduce a functions and firestore crash that shows in release mode only on iOS

The actual issue is/was upstream, and releases are rolling out now, but the issue we can respond to here is that this showed up in release mode easily but we never saw it since we were not testing release mode.

The main change here is just to add release mode build/test scripts for iOS, then augment the iOS CI check to run a buildmode matrix

Related issues

Release Summary

No release will be triggered by any commits in this PR, the actual fix was upstream and I filed pick requests that have already been integrated for future upstream releases

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

With the commit on this PR you can run these and see the crash, until the upstream patch is integrated in our e2e app

yarn test:ios:build:release
yarn test:ios:test:release

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Feb 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2025 4:33pm

@mikehardy mikehardy marked this pull request as draft February 25, 2025 00:00
@mikehardy mikehardy added plugin: functions Firebase Cloud Functions Needs Attention labels Feb 25, 2025
@mikehardy mikehardy force-pushed the ios-release-mode-functions-crash branch from c1030bb to d529c19 Compare March 20, 2025 13:42
@mikehardy mikehardy changed the title fix(functions, ios): repair release mode crash in functions test(ios): test iOS in release mode in addition to debug Mar 20, 2025
@mikehardy mikehardy added platform: ios Type: Testing Issues or PRs relating to testing (the library or user code testing) impact: crash Behaviour causing app to crash. Workflow: Pending Merge Waiting on CI or similar and removed plugin: functions Firebase Cloud Functions Needs Attention labels Mar 20, 2025
@mikehardy
Copy link
Collaborator Author

Perfect! Before adding the last commit that includes the upstream patch that fixes the issue, I wanted to make sure it reproduced the crash in testing since that is now the whole point of this PR. It did, here's what happened in the iOS release mode test:


  error: The app has crashed, see the details below:
  
  Exception in HostFunction: -[NSNull length]: unrecognized selector sent to instance 0x1e4b8f140
  
  Error: Exception in HostFunction: -[NSNull length]: unrecognized selector sent to instance 0x1e4b8f140
  (
  	0   Detox                               0x0000000103452514 +[NSThread(DetoxUtils) dtx_demangledCallStackSymbols] + 36
  	1   Detox                               0x0000000103455520 __DTXHandleCrash + 668
  	2   Detox                               0x0000000103456048 __dtx_terminate() + 376
  	3   libc++abi.dylib                     0x00000001802a6c50 std::__terminate(void (*)()) + 12
  	4   libc++abi.dylib                     0x00000001802a6c00 std::terminate() + 52
  	5   libdispatch.dylib                   0x000000018017198c _dispatch_client_callout + 36
  	6   libdispatch.dylib                   0x0000000180179b10 _dispatch_lane_serial_drain + 960
  	7   libdispatch.dylib                   0x000000018017a688 _dispatch_lane_invoke + 388
  	8   libdispatch.dylib                   0x0000000180185a84 _dispatch_root_queue_drain_deferred_wlh + 276
  	9   libdispatch.dylib                   0x00000001801850d0 _dispatch_workloop_worker_thread + 448
  	10  libsystem_pthread.dylib             0x0000000102c07b74 _pthread_wqthread + 284
  	11  libsystem_pthread.dylib             0x0000000102c06934 start_wqthread + 8
  )

So these won't slip through again

@mikehardy
Copy link
Collaborator Author

mikehardy commented Mar 20, 2025

iOS release mode e2e appears to have become stuck:

Thu, 20 Mar 2025 14:24:21 GMT
          ✔ resolves a started instance of a ScreenTrace
Thu, 20 Mar 2025 14:39:38 GMT
Error: The operation was canceled.

Going to check logs and see if I can determine why (was it a crash?) while re-running (was it a flake?)

edit: unable to check logs, artifacts were uploaded without matrix name in them so they collided and release mode artifact wouldn't upload, no simulator log to check. Re-running now with a fix for that so we can inspect in future

@mikehardy
Copy link
Collaborator Author

mikehardy commented Mar 20, 2025

Test failure is sendSignInLinkToEmail - instrumented with RNFBTest searchable string in Simulator log for future runs here

Then a secondary is that during the re-run it hangs on test after ✔ resolves a started instance of a ScreenTrace - that appears to be the final test in the perf.e2e.js suite and was successful, next suite is messaging.e2e.js and I'm not sure why it didn't work as there was no simulator log uploaded yet due to an error in github workflow yaml, hopefully next Simulator.log has more info

this immediately reproduces a functions crash
- note that you still need the packager started, auth tests use a continueUrl
  that hits 8081, and tests will fail if the port refuses connection
This fixes a null-handling crash bug upstream that we trigger in
functions and firestore

It is preseent in 0.77.1, and 0.78.0
It is fixed upstream in 0.77.2 and 0.78.1 and 0.79.0

This patch may be dropped once one of those versions with the fixed
is adopted here

The crash would not be triggered in our "other" platform support as
those don't rely on the native null handling in functions and firestore
@mikehardy mikehardy force-pushed the ios-release-mode-functions-crash branch from 5c18eb6 to 94da8dc Compare March 21, 2025 16:29
@mikehardy mikehardy merged commit 4bd883f into main Mar 21, 2025
19 of 21 checks passed
@mikehardy mikehardy deleted the ios-release-mode-functions-crash branch March 21, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact: crash Behaviour causing app to crash. platform: ios Type: Testing Issues or PRs relating to testing (the library or user code testing)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants