Better support for exceptions

[dcramer]

I need to dig into this more, so this ticket is mostly investigate, but the gist is that a formatted traceback might look something like this:

org.springframework.security.authentication.AuthenticationServiceException: Authentication failed 
at com.foobar.core.security.processor.AuthenticationProcessorImpl.authenticate(AuthenticationProcessorImpl.java:88)  

Where in Sentry, we end up with a somewhat-incorrect traceback:

Caused by: org.springframework.security.authentication.AuthenticationServiceException ("Authentication failed") ? at line -1
com.foobar.core.security.processor.AuthenticationProcessorImpl ? in authenticate at line 88

The big things here being:

1. Caused by should actually be part of the Exception interface
2. We're not recording a filename (though I'm not sure whats all available there)

There might be some additional things to clean up here as well.

Of note, I'm going to change how Java (and more importantly language-based) tracebacks look so they feel more natural to each individual platform.

[roam]

I think (been a while) the main reason the "Caused by" is actually included is to clarify wrapped exceptions.

Imagine the following stacktrace in which a FileNotFoundException is caught and rethrown, wrapped in a RuntimeException:

java.lang.RuntimeException: Could not open configuration at "/location"
    at com.foo.something.Else.openConfig
    at com.foo.something.Doing.boot
    ...
Caused by: FileNotFoundException: file "/location"
    at com.foo.fs.Loader.load
    ...

The second part is pretty crucial in determining the actual cause so we snuck it in there. Any ideas?

[dcramer]

@roam is that specifically the culprit (does it exist somewhere else in the stacktrace)?

[roam]

It's not a part of a standard Java stacktrace but inserted by the client code.

So filename is something like com.foo.TheClass (as close as you can get to a filename in Java) for the stacktrace frames itself, but it is set to the exception's message for the first record we manually insert to keep track of the "Caused by" message. That's also the "frame" with lineno set to -1.

If we were able to pass a message for a the stacktrace interface or multiple exception interfaces, we could remove this work-around without dropping pretty important information.

[dcramer]

@roam is it true that all frames in Java can have a message attached to them?

[dcramer]

Actually a followup here, would both stacktraces be the same and just a different exception message/value?

[roam]

Here's a typical stacktrace:

Exception in thread "main" java.lang.IllegalStateException: A book has a null property
        at com.example.myproject.Author.getBookIds(Author.java:38)
        at com.example.myproject.Bootstrap.main(Bootstrap.java:14)
Caused by: java.lang.NullPointerException
        at com.example.myproject.Book.getId(Book.java:22)
        at com.example.myproject.Author.getBookIds(Author.java:35)
        ... 1 more

This exception (IllegalStateException) is logged: logger.error("Could not get books", e).

What we end up with in the client is:

• the message Could not get books
• a Throwable of type IllegalStateException which contains the message A book has a null property, the stacktrace elements and the optional cause of the exception (i.e. the NullPointerException).

Each at com.example... is a stacktrace element which is added as a frame of Sentry's Stacktrace interface. The only information those elements contain are the name of the class, the name of the method and the line number. So no, not every frame needs a message; adding a message attribute to the Stacktrace interface would probably be the wrong way to go.

In the above scenario we're now adding the IllegalStateException as Sentry's Exception interface and the stacktrace elements of both exceptions to the Stacktrace interface, with an extra frame in between to get our "Caused by: java.lang.NullPointerException" back.

Ideally we'd be able to add both exceptions through the Exception interface.

[dcramer]

We could extend Exception to allow for a cause/culprit exception with the intention to handle composites.

Python has some odd things like this sometimes too, where its like:

An exception FOO happened, I caught it, and I reraised it as BAR.

What if the exception interface became:

{
    "type": ...,
    "value": ...,
    "cause": {
        "type": ...,
        "value": ...
    }
}

I'm not sure cause is the best word here for the internals. Thoughts?

[oliverhenlich]

Hi David.

Thanks again for following this up.

One of the most useful things we would like to see is the plain/raw unprocessed full java stack trace in the sentry ui. That would allow us to paste it in any popular java ide and have full visibility and navigatability of everything that happened leading to the error. At the moment the stack trace show in the ui is so different from a standard java exceptions that it cannot be used in the manner I described above.

I can provide example code for getting the full stack trace with all nested exceptions as a string if required.

Cheers
Oliver

On 21/03/2013, at 9:21, "David Cramer" <notifications@github.commailto:notifications@github.com> wrote:

We could extend Exception to allow for a cause/culprit exception with the intention to handle composites.

Python has some odd things like this sometimes too, where its like:

An exception FOO happened, I caught it, and I reraised it as BAR.

What if the exception interface became:

{
"type": ...,
"value": ...,
"cause": {
"type": ...,
"value": ...
}
}

I'm not sure cause is the best word here for the internals. Thoughts?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/43#issuecomment-15200933.

[roam]

@dcramer That looks perfect!

Wrapping exceptions in Java is far more common, especially with frameworks like Spring that like to turn checked into unchecked exceptions. And yeah, cause... original might fit the bill a bit better.

[vvasabi]

My $0.02 here. The term cause is inline with the Java API, see Throwable#getCause(). It is also quite common in Java to have multiple levels of nested exceptions. For example:

org.apache.myfaces.view.facelets.el.ContextAwareELException ("javax.el.ELException: Error reading 'var' on type foo.Object
    org.apache.myfaces.view.facelets.el.ContextAwareTagValueExpression in getValue at line 104 (application)
    javax.faces.component._DeltaStateHelper in eval at line 266 (application)
    javax.faces.component.UIComponentBase in isRendered at line 1187 (application)
    ...
Caused by: org.springframework.dao.InvalidDataAccessResourceUsageException ("Unknown column 'object.var' in 'field list'; SQL [n/a];")
    org.springframework.orm.hibernate3.SessionFactoryUtils in convertHibernateAccessException at line 635 (application)
    org.springframework.orm.jpa.vendor.HibernateJpaDialect in translateExceptionIfPossible at line 105 (application)
    ...
Caused by: org.hibernate.exception.SQLGrammarException: Unknown column 'object.var' in 'field list'
    org.hibernate.exception.internal.SQLExceptionTypeDelegate.convert(SQLExceptionTypeDelegate.java:82)
    org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:49)
    ...
Caused by: com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException ("Unknown column 'object.var' in 'field list'")
    sun.reflect.NativeConstructorAccessorImpl in newInstance0 at line -2 (application)
    sun.reflect.NativeConstructorAccessorImpl in newInstance at line 57 (application)
    sun.reflect.DelegatingConstructorAccessorImpl in newInstance at line 45 (application)
    ...

I would imagine Java developers like to see all wrapped exceptions, even though some may be redundant. In this case, would the JSON request object support nested objects as well? For example:

{
    "type": ...,
    "value": ...,
    "cause": {
        "type": ...,
        "value": ...,
        "cause": {
            "type": ...,
            "value": ...
        }
    }
}

As a side note, because of nested exceptions, sometimes stack trace messages from Java can be super long. The example above actually had something like 500 lines in its entirety. To make the message easier to read, it would be awesome if Sentry’s UI allows additional frames to be toggleable.

[oliverhenlich]

Hi Guys.

Thanks David for creating this ticket for me.

One of the most useful things we would like to see is the plain/raw unprocessed full java stack trace in the sentry ui. That would allow us to paste it in any popular java ide and have full visibility and navigatability of everything that happened leading to the error. At the moment the stack trace show in the ui is so different from a standard java exceptions that it cannot be used in the manner I described above.

I can provide example code for getting the full stack trace with all nested exceptions as a string if required.

Cheers
Oliver

[dcramer]

It almost feels like we might need to rethink stacktrace + exception separation here.

The gist of it as I see it is like this:

• At the high level it's still a single exception
• It contains many exceptions
• Each exception contains an (optional) stacktrace

We could provide a new interface which simply represents this kind of construct, and start moving things in that direction to better handle these cases.

[ColinHebert]

If I may ask, why is there a separation between stacktrace and exception in the first place?

I understand that you could have an exception without stacktrace under some circumstances, but a stacktrace without an exception seems weird to me.