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

Provide RequestHeaders to ServerErrorHandler #4037

Merged
merged 3 commits into from
Jan 25, 2022

Conversation

trustin
Copy link
Member

@trustin trustin commented Jan 20, 2022

Motivation:

A user sometimes wants to generate an error response differently
depending on a certain request header value. For example:

  • Include a certain request header value in an error response for easier
    troubleshooting; or
  • Provide a different level of detail for the requests that contain
    a certain header.

Modifications:

  • Added an optional RequestHeaders parameter to ServerErrorHandler
    methods.
  • Modified Http{1,2}RequestDecoder so it constructs a RequestHeaders
    as early as possible, so that ServerErrorHandler is given with a
    non-null RequestHeaders in most cases.
  • Updated CustomServerErrorHandlerTest to demonstrate this feature.

Result:

  • (new feature) A user can generate an error response differently
    depending on the request headers.
  • (breaking changes) The method signatures of ServerErrorHandler has
    been changed.

@trustin trustin added this to the 1.14.0 milestone Jan 20, 2022
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks, @trustin!

@trustin trustin force-pushed the expose_headers_in_error_handler branch from 6c2c7c4 to 12626aa Compare January 24, 2022 06:38
Motivation:

A user sometimes wants to generate an error response differently
depending on a certain request header value. For example:

- Include a certain request header value in an error response for easier
  troubleshooting; or
- Provide a different level of detail for the requests that contain
  a certain header.

Modifications:

- Added an optional `RequestHeaders` parameter to `ServerErrorHandler`
  methods.
- Modified `Http{1,2}RequestDecoder` so it constructs a `RequestHeaders`
  as early as possible, so that `ServerErrorHandler` is given with a
  non-null `RequestHeaders` in most cases.
- Updated `CustomServerErrorHandlerTest` to demonstrate this feature.

Result:

- (new feature) A user can generate an error response differently
  depending on the request headers.
- (breaking changes) The method signatures of `ServerErrorHandler` has
  been changed.
Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks, @trustin!

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you @trustin ! 👍 🙇 👍

@ikhoon ikhoon merged commit 1284b5e into line:master Jan 25, 2022
@trustin trustin deleted the expose_headers_in_error_handler branch June 14, 2023 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants