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

[WIP] Implement ServiceCall exception logging using HttpErrorHandler interface #2251

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

Conversation

yakovlevdmv
Copy link

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you signed the Lightbend CLA?
  • Have you added copyright headers to new files?
  • Have you updated the documentation?
  • Have you added tests for any changed functionality?

Fixes

Fixes #1091, #1375, #1398

Purpose

Provide the opportunity to override the ServiceRouter's exception logging.
The concept described in #1398:

... this part of the code shouldn't be handling errors at all, but let exceptions propagate up to be handled by the Play HttpErrorHandler...
-- @TimMoore

Todos:

  • Implement custom HttpErrorHandler and move existing code into it
  • Update the documentation
  • Tests

P.S. It's only the first version (PoC)

@yakovlevdmv yakovlevdmv requested a review from a team as a code owner September 9, 2019 22:59
@yakovlevdmv yakovlevdmv changed the title Implement ServiceCall exception logging using HttpErrorHandler interface [WIP] Implement ServiceCall exception logging using HttpErrorHandler interface Sep 11, 2019
@yakovlevdmv yakovlevdmv force-pushed the feature/service-router-custom-logging branch from 9c02b8a to 24f89b5 Compare September 19, 2019 12:13
@yakovlevdmv yakovlevdmv force-pushed the feature/service-router-custom-logging branch from 24f89b5 to 9d1905d Compare September 30, 2019 14:41
exceptionToResult(descriptor, filteredHeaders, e)
// logException(e, descriptor, call)
val result = processError(request, e)
Await.result(result, Duration.Inf)
Copy link
Contributor

Choose a reason for hiding this comment

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

There shouldn't be an Await here, as this will cause the thread to block. I believe the right way to do this would be to to replace recover above with recoverWith and return result.

@ignasi35 ignasi35 removed the request for review from a team April 16, 2020 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Allow overriding of ServiceRouter's exception logging
2 participants