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

[PLAT-5729 ] Fix malformed NSError from BSGJSONSerialization #987

Merged
merged 1 commit into from
Feb 1, 2021

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Jan 29, 2021

Goal

Fixes a low-occurrence crash observed in a customer dashboard

NSInvalidArgumentException: -[NSException domain]: unrecognized selector sent to instance 0x29b7d8930

0  CoreFoundation    ___exceptionPreprocess
1  libobjc.A.dylib   _objc_exception_throw
2  CoreFoundation    -[NSObject(NSObject) doesNotRecognizeSelector:]
3  CoreFoundation    ____forwarding___
4  CoreFoundation    ___forwarding_prep_0___
5  CoreFoundation    _CFErrorGetDomain
6  CoreFoundation    __CFErrorCopyUserInfoKeyFromCallBack
7  CoreFoundation    __CFErrorFormatDebugDescriptionAux
8  CoreFoundation    _userInfoKeyValueShow
9  CoreFoundation    -[__NSDictionaryI __apply:context:]
10 CoreFoundation    __CFErrorFormatDebugDescriptionAux
11 CoreFoundation    __CFErrorCreateDebugDescription
12 Foundation        -[NSError description]
13 CoreFoundation    -[NSObject(NSObject) _copyDescription]
14 CoreFoundation    _CFCopyDescription
15 CoreFoundation    ___CFStringAppendFormatCore
16 CoreFoundation    __CFStringCreateWithFormatAndArgumentsAux2
17 App               bsg_i_kslog_logObjC (BSG_KSLogger.m:259:29)
18 App               -[BSG_KSCrash captureThreads:depth:recordAllThreads:] (BSG_KSCrash.m:348:9)
19 App               -[BugsnagClient notify:handledState:block:] (BugsnagClient.m:980:24)
20 App               -[BugsnagClient notifyError:block:] (BugsnagClient.m:870:5)
21 App               +[Bugsnag notifyError:block:] (Bugsnag.m:132:9)

I was not able to reproduce the crash in a test case, but the root cause was an NSException being asked for its domain (which it doesn't have) so something that was expecting to be passed an NSError was being passed an NSException.

From the involvement of -[BSG_KSCrash captureThreads:depth:recordAllThreads:] it can be seen that the error being logged came from [BSGJSONSerialization JSONObjectWithData:options:error:]

So the root cause is an exception object being passed as the value for NSUnderlyingErrorKey and the system libraries trying to interpret it as an NSError in accordance with the documentation;

// Key in userInfo. A recommended standard way to embed NSErrors from underlying calls.
// The value of this key should be an NSError.
FOUNDATION_EXPORT NSErrorUserInfoKey const NSUnderlyingErrorKey;

Changeset

  • Removed offending value for NSUnderlyingErrorKey
  • Added test case and renamed test class / file to match convention (<ClassBeingTested>Tests)

Testing

Was not able to reproduce the crash.

Added test case to verify that NSUnderlyingErrorKey is an NSError, if provided.

@github-actions
Copy link

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size decreased by 72 bytes from 1,060,360 to 1,060,288 🎉

Generated by 🚫 Danger

@nickdowell nickdowell marked this pull request as ready for review January 29, 2021 14:58
@nickdowell nickdowell merged commit 2c68530 into next Feb 1, 2021
@nickdowell nickdowell deleted the nickdowell/fix-json-exception-errors branch February 1, 2021 08:43
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