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

Make exception mapper filters public #2050

Merged
merged 3 commits into from
Jan 13, 2022

Conversation

idelpivnitskiy
Copy link
Member

@idelpivnitskiy idelpivnitskiy commented Jan 11, 2022

Motivation:

Some users want to map exceptions earlier in the request processing
pipeline in order to observe a real HTTP status code in other filters.

Modifications:

  • Move DefaultHttpServerBuilder.ExceptionMapperServiceFilter ->
    io.servicetalk.http.api.HttpExceptionMapperServiceFilter;
  • Move io.servicetalk.grpc.netty.CatchAllHttpServiceFilter ->
    io.servicetalk.grpc.api.GrpcExceptionMapperServiceFilter;
  • Rename CatchAllHttpServiceFilterTest ->
    GrpcExceptionMapperServiceFilterTest;
  • Remove io.servicetalk.grpc.netty.GrpcUtils duplicate;
  • Add logging in GrpcExceptionMapperServiceFilter;

Result:

Users have access to our exception mapper implementations and can move
them around other filters in the processing pipeline.

Motivation:

Some users want to map exceptions earlier in the request processing
pipeline in order to observe a real HTTP status code in other filters.

Modifications:

- Move `DefaultHttpServerBuilder.ExceptionMapperServiceFilter` ->
`io.servicetalk.http.api.HttpExceptionMapperServiceFilter`;
- Move `io.servicetalk.grpc.netty.CatchAllHttpServiceFilter` ->
`io.servicetalk.grpc.api.GrpcExceptionMapperServiceFilter`;
- Rename `CatchAllHttpServiceFilterTest` ->
`GrpcExceptionMapperServiceFilterTest`;
- Remove `io.servicetalk.grpc.netty.GrpcUtils` duplicate;

Result:

Users have access to our exception mapper implementations and can move
them around other filters in the processing pipeline.
Copy link
Contributor

@tkountis tkountis left a comment

Choose a reason for hiding this comment

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

LGTM.
Curious if we can have a common filter for both, just different mapper.

@idelpivnitskiy
Copy link
Member Author

Not sure that will be possible due to their differences in behavior. I'm preparing another PR to enhance the grpc mapper, it currently misses a few cases.

@idelpivnitskiy idelpivnitskiy merged commit 967255d into apple:main Jan 13, 2022
@idelpivnitskiy idelpivnitskiy deleted the exception-mappers branch January 13, 2022 00:41
idelpivnitskiy added a commit that referenced this pull request Jan 13, 2022
Motivation:

Some users want to map exceptions earlier in the request processing
pipeline in order to observe a real HTTP status code in other filters.

Modifications:

- Move `DefaultHttpServerBuilder.ExceptionMapperServiceFilter` ->
`io.servicetalk.http.api.HttpExceptionMapperServiceFilter`;
- Move `io.servicetalk.grpc.netty.CatchAllHttpServiceFilter` ->
`io.servicetalk.grpc.api.GrpcExceptionMapperServiceFilter`;
- Rename `CatchAllHttpServiceFilterTest` ->
`GrpcExceptionMapperServiceFilterTest`;
- Remove `io.servicetalk.grpc.netty.GrpcUtils` duplicate;
- Add logging in `GrpcExceptionMapperServiceFilter`;

Result:

Users have access to our exception mapper implementations and can move
them around other filters in the processing pipeline.
idelpivnitskiy added a commit that referenced this pull request Jan 13, 2022
Motivation:

Some users want to map exceptions earlier in the request processing
pipeline in order to observe a real HTTP status code in other filters.

Modifications:

- Move `DefaultHttpServerBuilder.ExceptionMapperServiceFilter` ->
`io.servicetalk.http.api.HttpExceptionMapperServiceFilter`;
- Move `io.servicetalk.grpc.netty.CatchAllHttpServiceFilter` ->
`io.servicetalk.grpc.api.GrpcExceptionMapperServiceFilter`;
- Rename `CatchAllHttpServiceFilterTest` ->
`GrpcExceptionMapperServiceFilterTest`;
- Remove `io.servicetalk.grpc.netty.GrpcUtils` duplicate;
- Add logging in `GrpcExceptionMapperServiceFilter`;

Result:

Users have access to our exception mapper implementations and can move
them around other filters in the processing pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants