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

Adds a default send(Throwable) method to ServerResponse.java as the first step in providing an easy facility for reporting exceptions during HTTP processing #1378

Merged
merged 12 commits into from
Feb 18, 2020

Conversation

ljnelson
Copy link
Member

This PR, when ready, will add the ability to conveniently send a Throwable during HTTP processing.

Signed-off-by: Laird Nelson laird.nelson@oracle.com

…irst step in providing an easy facility for reporting exceptions during HTTP processing.

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
@ljnelson
Copy link
Member Author

See issue #878.

@ljnelson ljnelson self-assigned this Feb 12, 2020
@ljnelson ljnelson linked an issue Feb 12, 2020 that may be closed by this pull request
@ljnelson ljnelson added this to In Progress in Backlog Feb 12, 2020
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…odyWriter.java

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
@ljnelson
Copy link
Member Author

ljnelson commented Feb 13, 2020

  • Add default void send(Throwable) method to ServerResponse
  • Add ThrowableBodyWriter class that handles cases where stack traces should be output or not
  • Actually, consider moving the guts of ThrowableBodyWriter into ContentWriters appropriately
  • Figure out how to choose whether stack traces should be output or not, following the directive that such a choice should come from some kind of configuration (Needs separate issue and more discussion; see Provide a way to get configuration "into" MediaSupport #1392)
  • Add ThrowableBodyWriter, properly configured, to the tail of MediaSupport's list of default MessageBodyWriter implementations

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
… discussion

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…ter-furnishing method in ContentWriters.java per discussion

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…StackTrace method in ContentWriters.java per discussion

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…per direction

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
…per and into the write method per instruction

Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
Signed-off-by: Laird Nelson <laird.nelson@oracle.com>
@ljnelson ljnelson marked this pull request as ready for review February 13, 2020 23:48
Copy link
Member

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

I think it would be useful to have a configurable option for this writer:

  • send a message (maybe also configurable) "Internal server error"
  • send the stack trace across the network

Sending a stack trace by default is sometimes considered a security issue, as you leak internal information about your implementation. So it is really useful for development and troubleshooting, but should not be used in production.

@ljnelson
Copy link
Member Author

ljnelson commented Feb 14, 2020

Yes, the configuration will be a separate follow-on issue (see #1392), as Romain and I decided that getting configuration into MediaSupport etc. was worth its own issue. You'll notice in this PR as it stands that stack trace writing is disabled.

@ljnelson
Copy link
Member Author

Thanks; I'll merge this soon and then start work on #1392.

@tomas-langer
Copy link
Member

Thanks for explanation, approved.

@ljnelson ljnelson merged commit 25eb862 into helidon-io:master Feb 18, 2020
Backlog automation moved this from In Progress to Closed Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Backlog
  
Closed
Development

Successfully merging this pull request may close these issues.

Add exception writer to Helidon WebServer
3 participants