Skip to content

Commit

Permalink
fix: add missing fields to native errors in new arch (#43649)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vonovak authored and facebook-github-bot committed Apr 4, 2024
1 parent b309af7 commit 98b1331
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s
NSString *message = jsErrorDetails[@"message"];

auto jsError = createJSRuntimeError(runtime, [message UTF8String]);
jsError.asObject(runtime).setProperty(runtime, "cause", convertObjCObjectToJSIValue(runtime, jsErrorDetails));
for (NSString *key in jsErrorDetails) {
id value = jsErrorDetails[key];
jsError.asObject(runtime).setProperty(runtime, [key UTF8String], convertObjCObjectToJSIValue(runtime, value));
}

return jsError;
}
Expand Down

0 comments on commit 98b1331

Please sign in to comment.