Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When remote debugging is turned on, underlying error in promise rejection causes: NSError RCTJSONStringify: Invalid type in JSON write (NSError) #10506

Closed
neilsarkar opened this issue Oct 22, 2016 · 9 comments
Assignees
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@neilsarkar
Copy link
Contributor

Issue Description

Some native modules such as NSFileManager return an NSError that has an underlying NSError in the userInfo dictionary. React native handles this fine out of the box, but when the remote debugger is turned on, the websocket message sender chokes on attempting to stringify the second NSError into json.

I'm happy to help fix this, first time contributing so any pointers are welcome.

Steps to Reproduce / Code Snippets

I've posted a repository with the minimal reproduction case here https://github.com/superseriouscompany/react-native-error-repro

The steps are: first, turn on remote debugging. Then implement the following code:

// objc
RCT_EXPORT_METHOD(triggerError:(NSString *)foo
                  resolver:(RCTPromiseResolveBlock)resolve
                  rejecter:(RCTPromiseRejectBlock)reject)
{
  NSError *underlyingError = [NSError errorWithDomain:@"underlyingDomain" code:419 userInfo:nil];
  NSError *err = [NSError errorWithDomain:@"domain" code:68 userInfo:@{@"NSUnderlyingError": underlyingError}];

  reject(@"foo", @"bar", err);
}
// js
triggerError('anything').catch(err => { console.log(err)})

Expected Results

error is logged to console, execution continues.

Actual Results

### Additional Information - React Native version: 0.35.0 - Platform(s) (iOS, Android, or both?): iOS - Operating System (macOS, Linux, or Windows?): macOS
@lacker
Copy link
Contributor

lacker commented Oct 26, 2016

By the way I really appreciate that you are just sending a pull request to fix this! So props for that.

rozele pushed a commit to microsoft/react-native-windows that referenced this issue Oct 28, 2016
Summary:
Explain the **motivation** for making this change. What existing problem does the pull request solve?

See facebook/react-native#10506. A native `NSError` with `NSUnderlyingErrorKey` set causes a JSON stringify error from the websocket dispatcher if remote debugging is enabled.

**Test plan (required)**

I'm not familiar with the react native testing framework. Happy to add a test for this if someone can point me to where this part of the codebase is exercised :)

I did some spot checks with nil user dictionaries and nil underlying errors here. The case that this solves is testable using https://github.com/superseriouscompany/react-native-error-repro, specifically:

```objective-c
NSError *underlyingError = [NSError errorWithDomain:@"underlyingDomain" code:421 userInfo:nil];
NSError *err = [NSError errorWithDomain:@"domain" code:68 userInfo:@{@"NSUnderlyingError": underlyingError}];

reject(@"foo", @"bar", err);
```
Closes facebook/react-native#10507

Differential Revision: D4080802

Pulled By: lacker

fbshipit-source-id: 93a41d9e9a710e406a6ccac214a5617271b4bede
DanielMSchmidt pushed a commit to DanielMSchmidt/react-native that referenced this issue Jan 4, 2017
Summary:
Explain the **motivation** for making this change. What existing problem does the pull request solve?

See facebook#10506. A native `NSError` with `NSUnderlyingErrorKey` set causes a JSON stringify error from the websocket dispatcher if remote debugging is enabled.

**Test plan (required)**

I'm not familiar with the react native testing framework. Happy to add a test for this if someone can point me to where this part of the codebase is exercised :)

I did some spot checks with nil user dictionaries and nil underlying errors here. The case that this solves is testable using https://github.com/superseriouscompany/react-native-error-repro, specifically:

```objective-c
NSError *underlyingError = [NSError errorWithDomain:@"underlyingDomain" code:421 userInfo:nil];
NSError *err = [NSError errorWithDomain:@"domain" code:68 userInfo:@{@"NSUnderlyingError": underlyingError}];

reject(@"foo", @"bar", err);
```
Closes facebook#10507

Differential Revision: D4080802

Pulled By: lacker

fbshipit-source-id: 93a41d9e9a710e406a6ccac214a5617271b4bede
@fungilation
Copy link

I'm still hitting a similar error:

RCTJSONStringify() encountered the following error: Invalid type in JSON write (NSURL)

With NSURL in brackets instead.

I haven't been hitting this error until I upgraded RN from 0.37 to 0.41.

@miklschmidt
Copy link

@fungilation until or since? Because i still have it on 0.41.2.

@fungilation
Copy link

Until. I've never seen it on 0.37 and older. But that could just have been me testing less before.

@miklschmidt
Copy link

miklschmidt commented Mar 1, 2017

Okay, in my case it seems to be the AppTransportSecurityPolicy (or whatever it's called) that wasn't set to allow insecure connections. I had to hack RTCUtils.m:82 to print the JSON blob instead of the error. It seems like the userInfo object is the cause of the issue.

@fungilation
Copy link

In my case, I've already set AppTransportSecurityPolicy allowed insecure connections.

@miklschmidt
Copy link

Try doing the same thing, the json blob will tell you what's wrong. It's a workaround for now at least.

@neilsarkar
Copy link
Contributor Author

You can also disable Remote JS Debugging and read the logs from the XCode console in order to find out what the actual error is.

So I could write the same kind of patch for an NSError containing an NSURL that I did for an NSError containing an NSError, but that feels like playing a game of whack-a-mole...theoretically any object could be returned from an error and we would have to keep special casing in this function to serialize the error.

Anyone with a little more objective c knowledge have a suggestion for a way to generically stringify (or, worst case, strip) any type of object that could be attached to an error?

@neilsarkar
Copy link
Contributor Author

I was unable to reproduce with the following test:

- (void)testEncodingNSURL
{
  NSURL *url = [[NSURL alloc] initWithString:@"https://www.facebook.com"];
  
  NSError *err = [NSError errorWithDomain:@"domain" code:68 userInfo:@{@"url": url}];
  
  // An assertion on the full object would be too brittle since it contains an iOS stack trace
  // so we are relying on the behavior of RCTJSONParse, which is tested below.
  NSDictionary<NSString *, id> *jsonObject = RCTJSErrorFromNSError(err);
  NSString *jsonString = RCTJSONStringify(jsonObject, NULL);
  NSDictionary<NSString *, id> *json = RCTJSONParse(jsonString, NULL);
  XCTAssertEqualObjects(json[@"code"], @"EDOMAIN68");
  XCTAssertEqualObjects(json[@"message"], @"The operation couldn\u2019t be completed. (domain error 68.)");
  XCTAssertEqualObjects(json[@"domain"], @"domain");
  XCTAssertEqualObjects(json[@"userInfo"][@"url"], @"https://www.facebook.com");
}

so it may be a new or different error. can anyone provide steps for reproduction?

@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

5 participants