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 crash with nil value in valueForPath #41

Closed
wants to merge 1 commit into from

Conversation

Nikita2k
Copy link

If you have object[first] = nil it will crash (fatal error: unexpectedly found nil while unwrapping an Optional value), as of left we have non-optional type. You should change AnyObject to optional or remove it (but it will raise warning)

@ikesyo
Copy link
Owner

ikesyo commented Sep 22, 2015

Thanks for the pull request! However, those cases seems to be tested in our test cases, and the tests are passed without the change (the nested value should not be optional since the line has a optional binding pattern).

@ikesyo ikesyo closed this Sep 22, 2015
@ikesyo
Copy link
Owner

ikesyo commented Sep 22, 2015

So guard let nested: AnyObject = object[first] else is the same as the following:

let nested: AnyObject? = object[first]
if let nested = nested {
    ...
} else {
    return nil
}

@Nikita2k
Copy link
Author

@ikesyo In theory, yes. But in fact it crashes. Here is the radar:
http://www.openradar.me/22878681

You'll have crash too for the following object:

"data": "text"
e <| "data",
e <||? "optional_array" 

Can make a screencast for you.
Please, take a look at the radar and let me know, what you think.

@ikesyo
Copy link
Owner

ikesyo commented Sep 28, 2015

Thank you for the pointer to the rader. From what described in that rader, it seems that the crash happens only on device build (iPhone 5c), not on a simulator build, OK? And the actual crash is a EXC_BAD_INSTRUCTION, not the following you said at first?

(fatal error: unexpectedly found nil while unwrapping an Optional value)

@ikesyo
Copy link
Owner

ikesyo commented Sep 28, 2015

It would be great if you correctly offer that informations / assumptions. Those are really important to clarify the problem.

@Nikita2k
Copy link
Author

@ikesyo Sorry for not providing clear instructions. I'm able to reproduce this problem on simulator and any device.

The original problem

(fatal error: unexpectedly found nil while unwrapping an Optional value)

shows in debugger as EXC_BAD_INSTRUCTION, while "fatal error..." is printed in console.

I hope that helps

@ikesyo
Copy link
Owner

ikesyo commented Sep 28, 2015

Thank you for the inputs, and one more question. What version of Xcode (7.0/7.1b1/7.1b2) are you using? I've not tested this library with Xcode 7.1 beta yet. IMHO that should also be in the rader.

I'm able to reproduce this problem on simulator and any device.

If it is reproducable for simulators, the test suite should crash. Please run the tests on your environment.

@Nikita2k
Copy link
Author

I'm running Version 7.0 (7A220)
I will add this info to radar, thanks for tip!

I've tried to run your testcases, but I was not able to build, due to this errors:
screen shot 2015-09-28 at 18 36 18

As far as I can see, your test suit doesn't cover this case.
If you remove arrayOption from your JSON variable it will crash

@ikesyo
Copy link
Owner

ikesyo commented Sep 28, 2015

Run git submodule update --init.

@ikesyo
Copy link
Owner

ikesyo commented Sep 28, 2015

As far as I can see, your test suit doesn't cover this case.
If you remove arrayOption from your JSON variable it will crash

Missing keys should be covered with dictionaryOption of Person or optional of Group.

@Nikita2k
Copy link
Author

I can see your test passing, but in my project it is still crashing. Can't figure out, where the problem is

@ikesyo
Copy link
Owner

ikesyo commented Sep 29, 2015

@Nikita2k I did the steps of your rader with today's Xcode 7.0.1, and I can't reproduce the problem for both device and simulator. To reinstall Xcode might be help...

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

2 participants