Added handlers for unexpected errors (#1235)#1237
Conversation
| } else { | ||
| responseStatus = Status.INTERNAL_SERVER_ERROR; | ||
| // Create new message to prevent sending error information to client | ||
| ex = new RuntimeException("Oooops!.. Something went wrong :("); |
There was a problem hiding this comment.
This isn't exactly helpful when a user reports a problem. Can't we include some kind of informationa bout the nature of the error? Null Pointer Exception? Duplicate Key Exception? Something better than 'Oops! We died!'
There was a problem hiding this comment.
So, what I'd suggest is to add separate handlers for NPE and SQL exception (and we already have a handler for Duplicate Key - see if(ex instanceof DataIntegrityViolationException)) and leave others under a generic type of error with the "oops...". And as move forward, we could extend the list of specific error handlers on a need basis. @chrisknoll , what do you think of this?
There was a problem hiding this comment.
I'm trying to avoid messages that give no usuable information as to the nature of the problem (ie: the else of 'oops' is not acceptable'). what' I'm looking for here is the 'else' statement to provide some level of information about the nature of the exception. Is there any danger with instead of "Oppos!" we could get something like: 'An exception ocurred: ' + ex.getClass().getName()'
|
@chrisknoll , are you OK with the current approach? |
|
Yes, Looks good. Thank you. |
* Remove produces annotation + minor improvements (#1232) * Fix overlap logic. Allow event cohort periods that start within the target cohort era. Prior version required the event cohort era to start and end in the target cohort era. When collapsing, ensure start-end dates span at least a day. Use count_big when counting cohort events and pathways. Pad end dates to make non-inclusive. Added casts for impala Added temp-table cleanup. Fixes #779. * Omit cohort expression from list. - Fixes #219. * Added client cache directives to response header (#1231) * Removed legacy service which exposed server side request forgery issue (#1241) * Added handlers for unexpected errors (#1237) * Restored Kerberos auth support (#1253) * Fixes masquerading of credentials in PLE / PLP results (#1255) * Better quotes handling * exclude older sql render versions (#1259) * Fix compatability of /cohortdefinition/{id} endpoint (#1262) * fixes cycling dependencies (#1264) * Fix grouping problem in bq (#1267) (#1269) * Fixes failed Cohort Pathways on Impala (#1274) * fixes counts as bigint * fixes Pathways for BQ * Fixes save of Cohort Definitions when criteria requiring narrower CDM range are added (#1278) * Fixes timeout based invalidation of Execution Engine jobs (#1272) * Fixes Impala drivers (#1280) * Updates circe to newer release which resolves PLE issues on Impala * Merge fixes
5 participants
fixes #1235