Skip to content

Conversation

malandr2
Copy link
Contributor

@malandr2 malandr2 commented Oct 16, 2025

Description

Fix: [iOS] Prevent potential nil-key crashes in Native Ad module
This PR hardens the Native Module against potential crashes caused by inserting nil values into an NSMutableDictionary.

  • load: Method:

    • Added a nil check. If responseId is nil, we now reject the promise with an error instead of crashing the app when trying to set the value in _adHolders. Now matches Android implementation.
  • destroy: Method:

    • Added a nil check before calling removeObjectForKey: to prevent an NSInvalidArgumentException.
  • emitAdEvent:withData: Method:

    • Updated to use responseId ?: [NSNull null] to match the safe handling already used for responseId in the code.
    • Made the ad event a nonnull parameter.

Release Summary

Resolving crash NSInvalidArgumentException: *** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[1]

Checklist

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

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

  • 👉 Star this repo on GitHub ⭐️
  • 👉 Follow Invertase on Twitter

Copy link

docs-page bot commented Oct 16, 2025

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/react-native-google-mobile-ads~808

Documentation is deployed and generated using docs.page.

Copy link

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.34%. Comparing base (a34c7ba) to head (31ec193).
⚠️ Report is 176 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
- Coverage   43.72%   36.34%   -7.37%     
==========================================
  Files          30       37       +7     
  Lines         549      677     +128     
  Branches      151      176      +25     
==========================================
+ Hits          240      246       +6     
- Misses        309      431     +122     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dylancom dylancom merged commit 0ea211a into invertase:main Oct 17, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants