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.

Sign up for GitHub

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

Jump to bottom

[PLAT-8219] Fix crash when using BugsnagNetworkRequestPlugin alongside New Relic #1324

Merged
merged 1 commit into from
Mar 29, 2022
Merged

[PLAT-8219] Fix crash when using BugsnagNetworkRequestPlugin alongside New Relic #1324

merged 1 commit into from
Mar 29, 2022

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Mar 25, 2022 •
edited
Loading

Goal

Fix a crash that occurs when BugsnagNetworkRequestPlugin is linked into a project that uses the New Relic.

Sample crash: 
2022-03-25 12:00:40.842579+0000 NetApp[27482:2012467] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** -[NSProxy doesNotRecognizeSelector:URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:] called!'
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff203feba4 __exceptionPreprocess + 242
	1   libobjc.A.dylib                     0x00007fff201a1be7 objc_exception_throw + 48
	2   Foundation                          0x00007fff208184c2 +[NSProxy instanceMethodSignatureForSelector:] + 0
	3   CoreFoundation                      0x00007fff204030ac ___forwarding___ + 1433
	4   CoreFoundation                      0x00007fff204051d8 _CF_forwarding_prep_0 + 120
	5   CFNetwork                           0x00007fff23e164e5 _CFHostIsDomainTopLevelForCertificatePolicy + 29799
	6   Foundation                          0x00007fff207e24bb __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__ + 7
	7   Foundation                          0x00007fff207e23b3 -[NSBlockOperation main] + 98
	8   Foundation                          0x00007fff207e53c3 __NSOPERATION_IS_INVOKING_MAIN__ + 17
	9   Foundation                          0x00007fff207e1600 -[NSOperation start] + 785
	10  Foundation                          0x00007fff207e5d17 __NSOPERATIONQUEUE_IS_STARTING_AN_OPERATION__ + 17
	11  Foundation                          0x00007fff207e585c __NSOQSchedule_f + 182
	12  libdispatch.dylib                   0x00000001068bfa76 _dispatch_block_async_invoke2 + 83
	13  libdispatch.dylib                   0x00000001068b0a2c _dispatch_client_callout + 8
	14  libdispatch.dylib                   0x00000001068b73a6 _dispatch_lane_serial_drain + 845
	15  libdispatch.dylib                   0x00000001068b80f2 _dispatch_lane_invoke + 490
	16  libdispatch.dylib                   0x00000001068b71c7 _dispatch_lane_serial_drain + 366
	17  libdispatch.dylib                   0x00000001068b80bc _dispatch_lane_invoke + 436
	18  libdispatch.dylib                   0x00000001068c4472 _dispatch_workloop_worker_thread + 900
	19  libsystem_pthread.dylib             0x00007fff6da2845d _pthread_wqthread + 314
	20  libsystem_pthread.dylib             0x00007fff6da2742f start_wqthread + 15
)

Changeset

Uses the lighter-weight forwardingTargetForSelector: mechanism instead of implementing methodSignatureForSelector: and forwardInvocation:.

It's not entirely clear why this fixes the crash, but it does in the example app used, and is also faster 🚀

New Relic's session delegate implements forwardingTargetForSelector: rather than forwardInvocation: and the crash presumably arose because of the mismatch.

Testing

Reproduced the crash by creating an example app that uses BugsnagNetworkRequestPlugin, New Relic, and initiates a download task using Moya.

@github-actions
Copy link

Bugsnag.framework binary size did not change - 857,312 bytes

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  [ = ]       0    TOTAL

Generated by 🚫 Danger

@nickdowell nickdowell force-pushed the nickdowell/fix-newrelic-compatibility branch from af57df8 to c00351d Compare March 25, 2022 12:05
kstenerud
kstenerud approved these changes Mar 28, 2022
View reviewed changes
Copy link
Contributor

@kstenerud kstenerud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm cool with this as long as we're sure it's not going to break other library integrations.

This proxy stuff is always ugly...

@nickdowell nickdowell merged commit ca2c2c3 into next Mar 29, 2022
@nickdowell nickdowell deleted the nickdowell/fix-newrelic-compatibility branch March 29, 2022 07:29
@nickdowell nickdowell mentioned this pull request Mar 30, 2022
Merged
@nickdowell nickdowell mentioned this pull request Aug 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kattrali kattrali kattrali approved these changes

@kstenerud kstenerud kstenerud approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nickdowell @kstenerud @kattrali