Skip to content

Conversation

@rgomezp
Copy link

@rgomezp rgomezp commented Sep 27, 2025

Fix crash when transactionId is nil in transactionBegin

Description

This PR fixes a critical crash in the iOS Firestore transaction module where the app would crash with com.google.firebase.firestore.transaction when transactionId is nil. The crash occurs at line 156 in RNFBFirestoreTransactionModule.m when attempting to use a nil key in an NSMutableDictionary.

Crashed: com.google.firebase.firestore.transaction
0  CoreFoundation                 0x1e4cc -[NSDictionaryM setObject:forKeyedSubscript:] + 460
1  FitnessWolf                    0x7ed978 53-[RNFBFirestoreTransactionModule transactionBegin:::]_block_invoke + 156 (RNFBFirestoreTransactionModule.m:156)
2  FitnessWolf                    0x39be80 invocation function for block in -[FIRFirestore runTransactionWithOptions:block:dispatchQueue:completion:]::TransactionResult::RunUpdateBlock(std::1::shared_ptr<firebase::firestore::core::Transaction>, std::1::function<void (firebase::firestore::util::Status)>) + 329 (FIRFirestore.mm:329)
3  libdispatch.dylib              0x1aac dispatchcall_block_and_release + 32
4  libdispatch.dylib              0x1b584 dispatchclient_callout + 16
5  libdispatch.dylib              0x6560 dispatchcontinuation_pop + 596
6  libdispatch.dylib              0x5bd4 dispatchasync_redirect_invoke + 580
7  libdispatch.dylib              0x13db0 dispatchroot_queue_drain + 364
8  libdispatch.dylib              0x1454c dispatchworker_thread2 + 156
9  libsystem_pthread.dylib        0x9d0 pthreadwqthread + 232
10 libsystem_pthread.dylib        0xaac start_wqthread + 8

Root Cause

  • The transactionId parameter can be nil in certain edge cases
  • Calling [transactionId stringValue] on a nil NSNumber returns nil
  • Using nil as a key in NSMutableDictionary causes a crash in setObject:forKeyedSubscript:

Solution

  • Add early validation to check if transactionId is nil and return early with logging
  • Implement safe dictionary key handling by validating the string key before use
  • Return proper Firestore error instead of crashing when transactionId is invalid

Related issues

Fixes crash in com.google.firebase.firestore.transaction at line 156

Release Summary

  • iOS: Fixed crash when transactionId is nil in Firestore transactions
  • Breaking Change: No

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

Before Fix

  • App would crash with com.google.firebase.firestore.transaction when transactionId is nil
  • Crash occurs at line 156 in RNFBFirestoreTransactionModule.m
  • No error handling for invalid transaction IDs

After Fix

  • App no longer crashes when transactionId is nil
  • Proper error logging: "Error: transactionId is nil in transactionBegin"
  • Returns FIRFirestoreErrorCodeInvalidArgument error instead of crashing
  • Valid transactions continue to work normally

Test Cases

  1. Valid transaction: Should work as before
  2. Nil transactionId: Should log error and return early (no crash)
  3. Invalid transactionId: Should return proper Firestore error

Code Changes

// Early validation
if (transactionId == nil) {
  DLog(@"Error: transactionId is nil in transactionBegin");
  return;
}

// Safe dictionary key handling
NSString *transactionKey = [transactionId stringValue];
if (transactionKey != nil && !transactions[transactionKey]) {
  transactions[transactionKey] = transactionState;
} else {
  DLog(@"Invalid transactionId provided: %@", transactionId);
  *errorPointer = [NSError errorWithDomain:FIRFirestoreErrorDomain
                                      code:FIRFirestoreErrorCodeInvalidArgument
                                  userInfo:@{NSLocalizedDescriptionKey: @"Invalid transaction ID"}];
  return nil;
}

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

- Add nil check validation for transactionId parameter
- Add safe dictionary key handling to prevent NSMutableDictionary crash
- Return proper error instead of crashing when transactionId is invalid
- Fixes crash in com.google.firebase.firestore.transaction at line 156
@vercel
Copy link

vercel bot commented Sep 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
react-native-firebase Ready Ready Preview Comment Sep 27, 2025 7:29pm

@CLAassistant
Copy link

CLAassistant commented Sep 27, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Oct 27, 2025
@rgomezp
Copy link
Author

rgomezp commented Nov 4, 2025

I believe this has been fixed

@mikehardy
Copy link
Collaborator

Glad to hear this is no longer reproducible for you - I honestly can't see how it could happen, as a nil value for transactionId there would be counter to the nonnull specifier for the param in the native methods, and also seemingly impossible from the calling code. Maybe it was an upstream react-native issue, hard to say as the versions where you saw this aren't evident

Specifically, the transactionId is always generated from this generateTransactionId method, earlier in the JS layer before calling the native method, and this is the only place that native method is every called.

ID generation, I can't see how this would ever be null?

let transactionId = 0;
/**
* Uses the push id generator to create a transaction id
* @returns {number}
* @private
*/
const generateTransactionId = () => transactionId++;

Where that ID is generated, and the native method is called using the ID:

const id = generateTransactionId();
const meta = {
id,
updateFunction,
stack: new Error().stack.split('\n').slice(2).join('\n'),
};
this._pending[id] = {
meta,
transaction: new FirestoreTransaction(this._firestore, meta),
};
return new Promise((resolve, reject) => {
this._firestore.native.transactionBegin(id);

Proof that this is the only place that native method is ever called:

https://github.com/search?q=repo%3Ainvertase%2Freact-native-firebase%20transactionBegin&type=code

Obviously if this shows up again please let us know, otherwise I'll close this as it appears unneeded

@mikehardy mikehardy closed this Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants