iOS native module rejections are not `instanceof Error` #41950

vonovak · 2023-12-14T22:30:34Z

Description

Promise rejections coming from native modules on iOS are not instanceof Error. This is not in line with Android, and with what I believe is expected.

To reproduce, build for new arch on ios and run

Linking.openURL('sfslfjsd').catch(e => {
	console.warn({isInstanceOf: e instanceof Error});
});

I expect to see true but instead it's false

Steps to reproduce

npx react-native init bugrepro
RCT_NEW_ARCH_ENABLED=1 pod install

add

Linking.openURL('sfslfjsd').catch(e => {
	console.warn({isInstanceOf: e instanceof Error});
});

React Native Version

0.73.0

Affected Platforms

Runtime - iOS

Areas

TurboModule - The New Native Module System

Output of `npx react-native info`

System:
  OS: macOS 14.2
  CPU: (10) arm64 Apple M1 Max
  Memory: 101.94 MB / 64.00 GB
  Shell:
    version: 5.2.15
    path: /opt/homebrew/bin/bash
Binaries:
  Node:
    version: 21.2.0
    path: /opt/homebrew/bin/node
  Yarn:
    version: 1.22.19
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.2.3
    path: /opt/homebrew/bin/npm
  Watchman:
    version: 2023.09.04.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.12.1
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.0
      - iOS 17.0
      - macOS 14.0
      - tvOS 17.0
      - watchOS 10.0
  Android SDK:
    API Levels:
      - "24"
      - "28"
      - "29"
      - "30"
      - "31"
      - "32"
      - "33"
      - "33"
      - "33"
      - "34"
    Build Tools:
      - 28.0.3
      - 29.0.2
      - 30.0.2
      - 30.0.3
      - 31.0.0
      - 32.0.0
      - 33.0.0
      - 33.0.1
      - 34.0.0
    System Images:
      - android-19 | Google APIs ARM EABI v7a
      - android-21 | Google APIs ARM 64 v8a
      - android-22 | Google APIs ARM 64 v8a
      - android-23 | Intel x86 Atom
      - android-23 | Google APIs ARM 64 v8a
      - android-23 | Google APIs ARM EABI v7a
      - android-23 | Google APIs Intel x86 Atom_64
      - android-24 | ARM 64 v8a
      - android-24 | Google APIs ARM 64 v8a
      - android-25 | Google APIs ARM 64 v8a
      - android-28 | ARM 64 v8a
      - android-29 | Google APIs ARM 64 v8a
      - android-30 | Google Play ARM 64 v8a
      - android-31 | ARM 64 v8a
      - android-31 | Google Play ARM 64 v8a
      - android-32 | Google Play ARM 64 v8a
      - android-34 | Google APIs ARM 64 v8a
    Android NDK: 22.1.7171670
IDEs:
  Android Studio: Not Found
  Xcode:
    version: 15.0.1/15A507
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.7
    path: /usr/bin/javac
  Ruby:
    version: 2.7.8
    path: /opt/homebrew/opt/[email protected]/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.0
    wanted: 0.73.0
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

not applicable

Reproducer

https://github.com/vonovak/bugrepro

Screenshots and Videos

No response

The text was updated successfully, but these errors were encountered:

kelset · 2024-01-02T11:28:03Z

cc @cipolleschi - sounds like it's your area?

cipolleschi · 2024-01-03T10:04:06Z

Thanks for reporting this. We have a workstream this half to fix these inconsistencies, I'll put the issue in those tasks. We will try to address all of them before 0.74.

zhongwuzw · 2024-01-05T13:38:06Z

@cipolleschi Hi, can you help review this PR #41955? it should fix this issue.

cipolleschi · 2024-01-08T15:11:20Z

@zhongwuzw reviewed!

Summary: Fixes facebook#41950 . bypass-github-export-checks ## Changelog: [IOS] [FIXED] - Fixes Reject error kind in Fabric Pull Request resolved: facebook#41955 Test Plan: reject returns `Error`. Reviewed By: javache Differential Revision: D52622682 Pulled By: cipolleschi fbshipit-source-id: 726e68d968d03505748191263b7e6b75a068c130

Summary: With 0.74.rc-5 one bug which related to errors was fixed (#41950). However, the fix introduced another one: the shape of Error objects that come from native modules has changed. This PR attempts to fix that, though it's not (yet) doing it in a way that would be 100% compatible with the old arch. The problem was observed on iOS, not sure what the situation is on Android but believe it's okay there. edit: on Android, there are no issues but the `message` field is enumerable, so that part is different from ios (see logs below). Consider this code, where `error` is produced from a promise rejection inside of a native module. ```ts console.log( 'own properties: ', JSON.stringify(Object.getOwnPropertyNames(error), null, 2), ); console.log( 'own enumerable properties: ', JSON.stringify(Object.entries(error), null, 2), ); ``` These are the results for <details> <summary>Old architecture</summary> ``` LOG Running "google-one-tap-example" with {"rootTag":1,"initialProps":{}} LOG own properties: [ "stack", "code", "message", "domain", "userInfo", "nativeStackIOS" ] LOG own enumerable properties: [ [ "code", "-5" ], [ "message", "RNGoogleSignIn: The user canceled the sign in request., Error Domain=com.google.GIDSignIn Code=-5 \"The user canceled the sign-in flow.\" UserInfo={NSLocalizedDescription=The user canceled the sign-in flow.}" ], [ "domain", "com.google.GIDSignIn" ], [ "userInfo", { "NSLocalizedDescription": "The user canceled the sign-in flow." } ], [ "nativeStackIOS", [ "0 ReactTestApp 0x0000000102f4a6d8 RCTJSErrorFromCodeMessageAndNSError + 112", "1 ReactTestApp 0x0000000102eeedd0 __41-[RCTModuleMethod processMethodSignature]_block_invoke_2.73 + 152", "2 ReactTestApp 0x0000000102e2ae24 +[RNGoogleSignin rejectWithSigninError:withRejector:] + 548", "3 ReactTestApp 0x0000000102e2aa8c -[RNGoogleSignin handleCompletion:serverAuthCode:withError:withResolver:withRejector:fromCallsite:] + 184", "4 ReactTestApp 0x0000000102e2a8e0 -[RNGoogleSignin handleCompletion:withError:withResolver:withRejector:fromCallsite:] + 236", "5 ReactTestApp 0x0000000102e28628 __40-[RNGoogleSignin signIn:resolve:reject:]_block_invoke_2 + 100", "6 ReactTestApp 0x0000000102dc9d80 __35-[GIDSignIn addCompletionCallback:]_block_invoke_2 + 132", ... ] ] ] ``` </details> <details> <summary>RN 74 rc-5 (with bridgeless on)</summary> ``` (NOBRIDGE) LOG Bridgeless mode is enabled (NOBRIDGE) LOG Running "google-one-tap-example" with {"rootTag":1,"initialProps":{"concurrentRoot":true},"fabric":true} (NOBRIDGE) LOG own properties: [ "stack", "message", "cause" ] (NOBRIDGE) LOG own enumerable properties: [ [ "cause", { "code": "-5", "message": "RNGoogleSignIn: The user canceled the sign in request., Error Domain=com.google.GIDSignIn Code=-5 \"The user canceled the sign-in flow.\" UserInfo={NSLocalizedDescription=The user canceled the sign-in flow.}", "nativeStackIOS": [ "0 ReactTestApp 0x00000001023a7b38 RCTJSErrorFromCodeMessageAndNSError + 112", "1 ReactTestApp 0x00000001026cf774 ___ZZN8facebook5react15ObjCTurboModule13createPromiseERNS_3jsi7RuntimeENSt3__112basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEEU13block_pointerFvU13block_pointerFvP11objc_objectEU13block_pointerFvP8NSStringSH_P7NSErrorEEENK3$_0clES4_RKNS2_5ValueEPSQ_m_block_invoke.57 + 332", "2 ReactTestApp 0x0000000102270958 +[RNGoogleSignin rejectWithSigninError:withRejector:] + 548", "3 ReactTestApp 0x00000001022705c0 -[RNGoogleSignin handleCompletion:serverAuthCode:withError:withResolver:withRejector:fromCallsite:] + 184", "4 ReactTestApp 0x0000000102270414 -[RNGoogleSignin handleCompletion:withError:withResolver:withRejector:fromCallsite:] + 236", "5 ReactTestApp 0x000000010226e15c __40-[RNGoogleSignin signIn:resolve:reject:]_block_invoke_2 + 100", "6 ReactTestApp 0x000000010220f328 __35-[GIDSignIn addCompletionCallback:]_block_invoke_2 + 132", ... ], "domain": "com.google.GIDSignIn", "userInfo": { "NSLocalizedDescription": "The user canceled the sign-in flow." } } ] ] ``` </details> <details> <summary>with the diff from this PR</summary> ``` (NOBRIDGE) LOG own properties: [ "stack", "message", "code", "nativeStackIOS", "domain", "userInfo" ] (NOBRIDGE) LOG own enumerable properties: [ [ "code", "-5" ], [ "nativeStackIOS", [ "0 ReactTestApp 0x000000010083b8f8 RCTJSErrorFromCodeMessageAndNSError + 112", "1 ReactTestApp 0x0000000100b63534 ___ZZN8facebook5react15ObjCTurboModule13createPromiseERNS_3jsi7RuntimeENSt3__112basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEEU13block_pointerFvU13block_pointerFvP11objc_objectEU13block_pointerFvP8NSStringSH_P7NSErrorEEENK3$_0clES4_RKNS2_5ValueEPSQ_m_block_invoke.57 + 332", "2 ReactTestApp 0x0000000100704718 +[RNGoogleSignin rejectWithSigninError:withRejector:] + 548", "3 ReactTestApp 0x0000000100704380 -[RNGoogleSignin handleCompletion:serverAuthCode:withError:withResolver:withRejector:fromCallsite:] + 184", "4 ReactTestApp 0x00000001007041d4 -[RNGoogleSignin handleCompletion:withError:withResolver:withRejector:fromCallsite:] + 236", "5 ReactTestApp 0x0000000100701f1c __40-[RNGoogleSignin signIn:resolve:reject:]_block_invoke_2 + 100", "6 ReactTestApp 0x00000001006a30e8 __35-[GIDSignIn addCompletionCallback:]_block_invoke_2 + 132", ... ] ], [ "domain", "com.google.GIDSignIn" ], [ "userInfo", { "NSLocalizedDescription": "The user canceled the sign-in flow." } ] ] ``` </details> You see there is a change compared to old arch because `message` is no longer own enumerable property. If that needs to change (I guess it should), it'd be nice if someone more familiar with JSI pointed me in the right direction. Even with this inconsistency, the PR is an improvement and would be nice to have this fix included in the next RC. This is output from Chrome's console for completeness, just to have something to compare to: ``` let err = new Error('hello') undefined Object.getOwnPropertyNames(err) > ['stack', 'message'] Object.entries(err) > [] ``` bypass-github-export-checks ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [IOS] [FIXED] - add missing fields to native errors in new arch For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #43649 Test Plan: Tested locally with an example app running RN 74-rc 5 Reviewed By: cortinico Differential Revision: D55690184 Pulled By: cipolleschi fbshipit-source-id: 60a857b9871af888dcd526782b5e6b73c07c051a

vonovak added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Dec 14, 2023

github-actions bot added the Platform: iOS iOS applications. label Dec 14, 2023

zhongwuzw mentioned this issue Dec 15, 2023

[iOS] Fixes Reject error kind in Fabric #41955

Closed

cortinico added the Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. label Dec 27, 2023

facebook-github-bot closed this as completed in 9525074 Jan 10, 2024

vonovak mentioned this issue Mar 26, 2024

fix: add missing fields to native errors in new arch #43649

Closed

cipolleschi mentioned this issue Apr 5, 2024

[0.74] fix: add missing fields to native errors in new arch reactwg/react-native-releases#199

Closed

vonovak mentioned this issue Apr 11, 2024

Android native module rejections are not instanceof Error #44050

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS native module rejections are not `instanceof Error` #41950

iOS native module rejections are not `instanceof Error` #41950

vonovak commented Dec 14, 2023 •

edited

Loading

kelset commented Jan 2, 2024

cipolleschi commented Jan 3, 2024

zhongwuzw commented Jan 5, 2024

cipolleschi commented Jan 8, 2024

iOS native module rejections are not instanceof Error #41950

iOS native module rejections are not instanceof Error #41950

Comments

vonovak commented Dec 14, 2023 • edited Loading

Description

Steps to reproduce

React Native Version

Affected Platforms

Areas

Output of npx react-native info

Stacktrace or Logs

Reproducer

Screenshots and Videos

kelset commented Jan 2, 2024

cipolleschi commented Jan 3, 2024

zhongwuzw commented Jan 5, 2024

cipolleschi commented Jan 8, 2024

iOS native module rejections are not `instanceof Error` #41950

iOS native module rejections are not `instanceof Error` #41950

vonovak commented Dec 14, 2023 •

edited

Loading

Output of `npx react-native info`