Add RecoverInterceptor as alternative to WithRecover #824
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds things to the handler signature (the other properties of
AnyRequest
, such asPeer
and HTTP method) and also provides the interceptor separately, so it can be more easily combined with other interceptors. Finally, the value of ignoringhttp.ErrAbortHandler
is unclear and adds a potential footgun in that a handler that panics with that value may inadvertently prevent capturing metrics or other side effects that are desirable for error-tracking in the face of a panic.WithRecover
remains backwards-compatible. But it is deprecated; users should prefer the newRecoverInterceptor
method and use it withWithInterceptors
.The most controversial parts of this change:
AnyRequest
as the way to pass request properties, even for streaming RPCs. Does this seem like a bad idea? This was done to avoid adding a new exported type and to avoid a long parameter list. One awkwardness with this approach is having areq.Any()
method that isn't useful for client and bidi streams and is awkward to populate for server streams.Thoughts?
Resolves #816.