From 6183c72450ce2c7098fa8b11a2cd208598dcfc8f Mon Sep 17 00:00:00 2001 From: Dan LaRocque Date: Thu, 25 Sep 2014 13:30:29 -0400 Subject: [PATCH] Allow WebApplicationExceptions in VertexResource 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. --- .../com/tinkerpop/rexster/VertexResource.java | 48 +++++++++++++------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/rexster-server/src/main/java/com/tinkerpop/rexster/VertexResource.java b/rexster-server/src/main/java/com/tinkerpop/rexster/VertexResource.java index 5489466b..a6c74635 100644 --- a/rexster-server/src/main/java/com/tinkerpop/rexster/VertexResource.java +++ b/rexster-server/src/main/java/com/tinkerpop/rexster/VertexResource.java @@ -572,6 +572,8 @@ private Response getVertexEdges(String graphName, String vertexId, String direct final JSONObject error = generateErrorObjectJsonFail(ex); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); } catch (WebApplicationException wae) { + logger.error(wae); + throw wae; } catch (Exception re) { logger.error(re); @@ -726,20 +728,22 @@ private Response postVertex(final String graphName, final String id, final boole } this.resultObject.put(Tokens.QUERY_TIME, sh.stopWatch()); - } catch (JSONException ex) { - rag.tryRollback(); + } catch (WebApplicationException wae) { + logger.error(wae); + throw wae; + } catch (JSONException ex) { logger.error(ex); JSONObject error = generateErrorObjectJsonFail(ex); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); } catch (Exception re) { - rag.tryRollback(); - logger.error(re); JSONObject error = generateErrorObject(re.getMessage(), re); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); + } finally { + safeRollback(rag); } return Response.ok(this.resultObject).build(); @@ -844,20 +848,22 @@ private Response putVertex(final String graphName, final String id, final boolea } this.resultObject.put(Tokens.QUERY_TIME, sh.stopWatch()); - } catch (JSONException ex) { - rag.tryRollback(); + } catch (WebApplicationException wae) { + logger.error(wae); + throw wae; + } catch (JSONException ex) { logger.error(ex); JSONObject error = generateErrorObjectJsonFail(ex); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); } catch (Exception re) { - rag.tryRollback(); - logger.error(re); JSONObject error = generateErrorObject(re.getMessage(), re); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); + } finally { + safeRollback(rag); } return Response.ok(this.resultObject).build(); @@ -898,31 +904,45 @@ public Response deleteVertex(@PathParam("graphname") String graphName, @PathPara logger.info(msg); JSONObject error = generateErrorObject(msg); + throw new WebApplicationException(Response.status(Status.NOT_FOUND).entity(error).build()); } rag.tryCommit(); this.resultObject.put(Tokens.QUERY_TIME, sh.stopWatch()); - } catch (JSONException ex) { - - rag.tryRollback(); + } catch (WebApplicationException wae) { + logger.error(wae); + throw wae; + } catch (JSONException ex) { logger.error(ex); final JSONObject error = generateErrorObjectJsonFail(ex); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); } catch (Exception re) { - - rag.tryRollback(); - logger.error(re); final JSONObject error = generateErrorObject(re.getMessage(), re); throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(error).build()); + } finally { + safeRollback(rag); } return Response.ok(this.resultObject).build(); + } + /** + * Calls {@link RexsterApplicationGraph#tryRollback()} on the supplied parameter. + * Swallows any {@link java.lang.Throwable} produced by {@code tryRollback} after + * logging the exception at the {@code WARN} severity level. This method always + * returns and never throws, even if it fails. + */ + private static void safeRollback(RexsterApplicationGraph rag) { + try { + rag.tryRollback(); + } catch (Throwable t) { + logger.warn("Unable to rollback graph transaction", t); + } } private static String[] getLabelsFromRequest(JSONObject theRequestObject) {