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

Handle uncaught exceptions #171

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Oct 7, 2015

Here is my take on fixing #87.

0 CoreFoundation 0x0000000109479a75 __exceptionPreprocess + 165
1 libobjc.A.dylib 0x0000000109112bb7 objc_exception_throw + 45
2 CoreFoundation 0x000000010938503f -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 383
3 CoreFoundation 0x0000000109397d8b +[NSDictionary dictionaryWithObjects:forKeys:count:] + 59
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tab detected.

@supermarin
Copy link
Contributor

Thanks @0xced, this is awesome work!

Would you mind writing a feature for the crash as well?

@kylef
Copy link
Contributor

kylef commented Oct 7, 2015

I've just ran this this branch on my project throwing an exception and it doesn't catch my exception being thrown in XCTest since XCTest doesn't terminate the app but instead handle these and mark the test as failed.

UNCAUGHT_EXCEPTION_MATCHER only detects "Terminating app due to uncaught exception" where in my case xcodebuild prints:

`CCLFormat/NSAttributedStringFormatTests/NSAttributedString+CCLFormatTests.m:17: error: -[NSAttributedStringTests testAttributedFormatStringCanFormatAttributedString] : failed: caught "¯\_(ツ)_/¯", "¯\_(ツ)_/¯"`

Example Code:

- (void)testAttributedFormatStringCanFormatAttributedString {
    @throw [NSException exceptionWithName:@"¯\\_(ツ)_/¯" reason:@"¯\\_(ツ)_/¯" userInfo:nil];
}
Build settings from command line:
    SDKROOT = macosx10.11

=== BUILD TARGET NSAttributedStringFormat OF PROJECT NSAttributedStringFormat WITH CONFIGURATION Debug ===

Check dependencies

=== BUILD TARGET NSAttributedStringFormatTests OF PROJECT NSAttributedStringFormat WITH CONFIGURATION Debug ===

Check dependencies

Test Suite 'All tests' started at 2015-10-07 09:10:05.775
Test Suite 'NSAttributedStringFormatTests.xctest' started at 2015-10-07 09:10:05.777
Test Suite 'NSAttributedStringTests' started at 2015-10-07 09:10:05.777
Test Case '-[NSAttributedStringTests testAttributedFormatString]' started.
Test Case '-[NSAttributedStringTests testAttributedFormatString]' passed (0.006 seconds).
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatAttributedString]' started.
/Users/kyle/Projects/cocode/NSAttributedString-CCLFormat/NSAttributedStringFormatTests/NSAttributedString+CCLFormatTests.m:17: error: -[NSAttributedStringTests testAttributedFormatStringCanFormatAttributedString] : failed: caught "¯\_(ツ)_/¯", "¯\_(ツ)_/¯"
(
    0   CoreFoundation                      0x00007fff927d4bd2 __exceptionPreprocess + 178
    1   libobjc.A.dylib                     0x00007fff941484fa objc_exception_throw + 48
    2   NSAttributedStringFormatTests       0x0000000105b8d35a -[NSAttributedStringTests testAttributedFormatStringCanFormatAttributedString] + 314
    3   CoreFoundation                      0x00007fff927468cc __invoking___ + 140
    4   CoreFoundation                      0x00007fff9274675e -[NSInvocation invoke] + 286
    5   XCTest                              0x000000010585ab53 __24-[XCTestCase invokeTest]_block_invoke_2 + 159
    6   XCTest                              0x000000010588d026 -[XCTestContext performInScope:] + 184
    7   XCTest                              0x000000010585aaa3 -[XCTestCase invokeTest] + 169
    8   XCTest                              0x000000010585af3e -[XCTestCase performTest:] + 443
    9   XCTest                              0x0000000105858c0f -[XCTestSuite performTest:] + 377
    10  XCTest                              0x0000000105858c0f -[XCTestSuite performTest:] + 377
    11  XCTest                              0x0000000105858c0f -[XCTestSuite performTest:] + 377
    12  XCTest                              0x00000001058475c3 __25-[XCTestDriver _runSuite]_block_invoke + 51
    13  XCTest                              0x000000010586b736 -[XCTestObservationCenter _observeTestExecutionForBlock:] + 611
    14  XCTest                              0x000000010584750c -[XCTestDriver _runSuite] + 408
    15  XCTest                              0x00000001058480bb -[XCTestDriver _checkForTestManager] + 696
    16  XCTest                              0x000000010588e273 _XCTestMain + 628
    17  xctest                              0x0000000105838729 xctest + 5929
    18  libdyld.dylib                       0x00007fff90c945ad start + 1
)
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatAttributedString]' failed (0.421 seconds).
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatDescription]' started.
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatDescription]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatString]' started.
Test Case '-[NSAttributedStringTests testAttributedFormatStringCanFormatString]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatString]' started.
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatString]' passed (0.001 seconds).
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatAttributedString]' started.
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatAttributedString]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatDescription]' started.
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatDescription]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatMoreThanNineArgs]' started.
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatMoreThanNineArgs]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatString]' started.
Test Case '-[NSAttributedStringTests testPositionalAttributedFormatStringCanFormatString]' passed (0.000 seconds).
Test Case '-[NSAttributedStringTests testPositionalFormatStringCanFormatRepeatedArgs]' started.
Test Case '-[NSAttributedStringTests testPositionalFormatStringCanFormatRepeatedArgs]' passed (0.000 seconds).
Test Suite 'NSAttributedStringTests' failed at 2015-10-07 09:10:06.213.
     Executed 10 tests, with 1 failure (1 unexpected) in 0.431 (0.436) seconds
Test Suite 'NSAttributedStringFormatTests.xctest' failed at 2015-10-07 09:10:06.214.
     Executed 10 tests, with 1 failure (1 unexpected) in 0.431 (0.437) seconds
Test Suite 'All tests' failed at 2015-10-07 09:10:06.215.
     Executed 10 tests, with 1 failure (1 unexpected) in 0.431 (0.439) seconds

Save the above to output.txt and run cat output.txt | xcpretty -c.

@supermarin
Copy link
Contributor

Thanks @kylef !

Definitely worth adding a feature for both of these cases

@supermarin
Copy link
Contributor

@0xced it might be worth checking the behavior with Swift as well

@0xced
Copy link
Contributor Author

0xced commented Oct 7, 2015

This pull request specifically addresses uncaught exceptions but of course it seems worth also handling caught exceptions! Do you think caught exceptions should also include the full call stack?

@supermarin
Copy link
Contributor

@0xced I don't have a strong preference would it be 1 or 2 PRs.
If it doesn't add too much scope in this one it might be better to have it in :)

@kylef
Copy link
Contributor

kylef commented Oct 7, 2015

@0xced I'm unclear how you would get uncaught exceptions now that XCTest catches them internally. Are you using some other non-XCTest based testing framework?

@0xced
Copy link
Contributor Author

0xced commented Oct 7, 2015

@kylef Easy with some asynchronous code:

- (void) testUncaughtException
{
    XCTestExpectation *expectation = [self expectationWithDescription:@"I would expect that *all* exceptions are caught, but they aren’t…"];
    dispatch_async(dispatch_get_main_queue(), ^{
        [expectation fulfill];
        @throw [NSException exceptionWithName:@"Boom" reason:@"¯\\_(ツ)_/¯" userInfo:nil];
    });
    [self waitForExpectationsWithTimeout:1 handler:nil];
}

And here’s an in-depth explanation of why it’s not caught.

@kylef
Copy link
Contributor

kylef commented Oct 7, 2015

@0xced Thanks for clarifying, that makes sense. I have very few frameworks that are asynchronous and haven't came into this in a while.

@0xced
Copy link
Contributor Author

0xced commented Oct 8, 2015

Should I rebase on master and force push or is it going to make this PR very difficult to follow if I do so?

@supermarin
Copy link
Contributor

Please feel welcome to go ahead Sir 🚀

'-[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[0]',
[' 0 CoreFoundation 0x0000000109479a75 __exceptionPreprocess + 165',
' 1 libobjc.A.dylib 0x0000000109112bb7 objc_exception_throw + 45',
' 2 CoreFoundation 0x000000010938503f -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 383',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line is too long. [139/80]
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.


\t0 CoreFoundation 0x0000000109479a75 __exceptionPreprocess + 165
\t1 libobjc.A.dylib 0x0000000109112bb7 objc_exception_throw + 45
\t2 CoreFoundation 0x000000010938503f -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 383
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line is too long. [126/80]

@0xced
Copy link
Contributor Author

0xced commented Oct 8, 2015

I just rebased on master. Should I consider Hound comments? I see plenty of other parts where single quoted strings are used and lies are too long.

@supermarin
Copy link
Contributor

@0xced I've deleted some of them and @kattrali has worked recently on specifying this stuff better.
I'd say just keep going with your PR and when you're ready to merge we can review / decide

@kattrali
Copy link
Contributor

It looks fine to me

@Coder-256
Copy link

@supermarin What's the status on this?

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

6 participants