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

Fix UserInfo Crash #17488

Merged
merged 4 commits into from
Nov 17, 2021
Merged

Fix UserInfo Crash #17488

merged 4 commits into from
Nov 17, 2021

Conversation

illusaen
Copy link
Contributor

@illusaen illusaen commented Nov 17, 2021

Fixes a crash where cloning the userInfo of an NSError is causing a Core Data exception when trying to save the context.

2021-11-17 14:10:31:827 WordPress[1434:200461] Unresolved Core Data save error: Error Domain=NSCocoaErrorDomain Code=134060 "A Core Data error occurred." UserInfo={NSUnderlyingError=0x120604370 {Error Domain=NSCocoaErrorDomain Code=4866 "The data couldn’t be written because it isn’t in the correct format." UserInfo={NSUnderlyingError=0x120618770 {Error Domain=NSCocoaErrorDomain Code=4864 "This decoder will only decode classes that adopt NSSecureCoding. Class 'NWConcrete_nw_path' does not adopt it." UserInfo={NSDebugDescription=This decoder will only decode classes that adopt NSSecureCoding. Class 'NWConcrete_nw_path' does not adopt it.}}}}}

To test:

Prerequisites: iOS 15

  1. Go to a simple site.
  2. Put the device into Airplane Mode.
  3. Add an image block to a post.
  4. Select an image from the device.
  5. See that it doesn't crash and a "Failed to insert media. Please tap for options." message is displayed.

Regression Notes

  1. Potential unintended areas of impact
    Error should still exist and show that there was a networking error, just without crashing.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Use the above steps to test.

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@illusaen illusaen requested a review from twstokes November 17, 2021 17:46
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 17, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@twstokes twstokes self-assigned this Nov 17, 2021
@twstokes
Copy link
Contributor

twstokes commented Nov 17, 2021

Interestingly the crash doesn't occur (nor the errors) if the original NSError is used. Comparing the "custom" error to the original, only the descriptions differ (because that's what we inject). The NSUnderlyingError key in the userInfo dictionary appears to be the culprit. Possibly the fact that its value is an object is a clue.

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@ScoutHarris ScoutHarris self-requested a review November 17, 2021 19:45
@ScoutHarris ScoutHarris added this to the 18.8 milestone Nov 17, 2021
@twstokes
Copy link
Contributor

I also want to note that I failed to find any spots in code where we cared about anything more than the domain, code, and description when dealing with Media errors.

Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

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

✅ See that it doesn't crash and a "Failed to insert media. Please tap for options." message is displayed.

:shipit:

@twstokes twstokes merged commit b24e890 into develop Nov 17, 2021
@twstokes twstokes deleted the userinfo-crash branch November 17, 2021 20:05
@twstokes twstokes restored the userinfo-crash branch November 18, 2021 15:05
@twstokes twstokes deleted the userinfo-crash branch November 18, 2021 15:07
twstokes added a commit that referenced this pull request Nov 18, 2021
@twstokes twstokes added the Core Data Issues related to Core Data label Nov 18, 2021
@twstokes twstokes modified the milestones: 18.8, 18.7 ❄️ Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Data Issues related to Core Data [Type] Crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retrying failed media uploads without Internet connectivity leads to Core Data exception
3 participants