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

Allow WebApplicationExceptions in VertexResource #367

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dalaro
Copy link

@dalaro dalaro commented Sep 25, 2014

This PR covers thinkaurelius/titan#728.

Several WebApplicationExceptions thrown inside VertexResource methods
seemed to be unintentionally swallowed by enclosing catch clauses with
low specificity (e.g. Exception). These catchall clauses generally
swallow the WAE's status code and replace it with a generic 500
internal server error. This commit changes the catch clauses at the
bottom of three request methods to allow a WAE to propagate up the
stack instead of converting it into a generic 500.

This commit also introduces a try-catch with exception logging around
the RexsterApplicationGraph.tryRollback call made inside several of
these catch clauses. The point of this change is to make exceptions
encountered during rollback get logged and to allow the remainder of
the catch clause to attempt to execute even if rollback failed. I ran
into failing rollbacks when testing Titan 0.5.1-SNAPSHOT against
Rexster 2.6.0 without defining
Features.supportsThreadIsolatedTransactions, but I was blind to that
cause for a while because the exception message and trace were
suppressed.

Several WebApplicationExceptions thrown inside VertexResource methods
seemed to be unintentionally swallowed by enclosing catch clauses with
low specificity (e.g. Exception).  These catchall clauses generally
swallow the WAE's status code and replace it with a generic 500
internal server error.  This commit changes the catch clauses at the
bottom of three request methods to allow a WAE to propagate up the
stack instead of converting it into a generic 500.

This commit also introduces a try-catch with exception logging around
the RexsterApplicationGraph.tryRollback call made inside several of
these catch clauses.  The point of this change is to make exceptions
encountered during rollback get logged and to allow the remainder of
the catch clause to attempt to execute even if rollback failed.  I ran
into failing rollbacks when testing Titan 0.5.1-SNAPSHOT against
Rexster 2.6.0 without defining
Features.supportsThreadIsolatedTransactions, but I was blind to that
cause for a while because the exception message and trace were
suppressed.
@spmallette
Copy link
Member

Thanks dan...will merge as soon as we bump to snapshot. I'll go through the other resources as well and make them consistent given your changes.

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.

2 participants