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

ensure errors are logged in ExceptionHandler [no ticket; risk: low] #817

Merged
merged 1 commit into from
Nov 18, 2020

Conversation

davidangb
Copy link
Contributor

in the global ExceptionHandler for all routes, add a statement to log the exception in the catch-all case.

The global ExceptionHandler otherwise swallows these errors if they're not manually logged. In the case of some akka errors, such as when akka rejects a payload for being too big, this meant the exception was silently discarded.

See also broadinstitute/rawls#1316


Have you read CONTRIBUTING.md lately? If not, do that first.

I, the developer opening this PR, do solemnly pinky swear that:

  • I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
  • I've updated the RC_XXX release ticket with any manual steps required to release this change
  • I've updated the FISMA documentation if I've made any security-related changes, including auth, encryption, or auditing

In all cases:

  • Get two thumbsworth of review and PO signoff if necessary
  • Verify all tests go green
  • Squash and merge; you can delete your branch after this unless it's for a hotfix. In that case, don't delete it!
  • Test this change deployed correctly and works on dev environment after deployment

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 64.359% when pulling 5f897eb on da_logHandledExceptions into 02d25a0 on develop.

@davidangb
Copy link
Contributor Author

error in fiab-start that looks transient:

ERROR: for martha-proxy  Cannot start service martha-proxy: driver failed programming external connectivity on endpoint firecloud_martha-proxy_1 (e0085ec73dc022779662dee17dd1f528bc40d26642e8fcafbe52e207b4ec901d): Error starting userland proxy: listen tcp 0.0.0.0:32800: bind: address already in use

jenkins retest.

@@ -33,6 +34,12 @@ object FireCloudApiService {
case withErrorReport: FireCloudExceptionWithErrorReport =>
complete(withErrorReport.errorReport.statusCode.getOrElse(StatusCodes.InternalServerError) -> withErrorReport.errorReport)
case e: Throwable =>
// so we don't log the error twice when debug is enabled
if (logger.underlying.isDebugEnabled) {
logger.debug(e.getMessage, e)
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidangb davidangb merged commit 4ffd833 into develop Nov 18, 2020
@davidangb davidangb deleted the da_logHandledExceptions branch November 18, 2020 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants