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

Prevent crashing during non critical logging at rotation #1770

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

foxware00
Copy link
Contributor

As mentioned in the Slack channel.

I receive a crash during device rotation of certain screens. It happens when logging out new traits inside the NSStringFromASPrimitiveTraitCollection method during the ASViewController.mm#propagateNewTraitCollection calls.

The controller in question is an old UIStoryboard, I'm not sure of the reason for the nil preferredContentSizeCategory being nil, but given it's just a log, it shouldn't be crashing the app.

This PR simply adds a null check to the String conversion method, I'm not sure if someone can shed more light on whether or not this is appropriate, or the root cause might be cause for concern? @Adlai-Holler @nguyenhuy

@claassistantio
Copy link

claassistantio commented Feb 4, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this!

@nguyenhuy
Copy link
Member

To answer your questions, I think the null check is appropriate because the log shouldn't crash the app. And it's weird that you didn't hit assertions around that property (ASDisplayNodeCAssertPermanent).

I can't say for sure why the property is nil. Perhaps that's because the old storyboard causes the construction of a trait collection (perhaps on an old iOS version?) that doesn't set a value to that property, or did you construct it yourself? Either way, you should be able to tell by breaking at that line and trace back the origin of that struct.

@raycsh017 raycsh017 merged commit 7798440 into TextureGroup:master Feb 11, 2020
raycsh017 added a commit that referenced this pull request Feb 11, 2020
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.

None yet

4 participants