Skip to content

Fix ErrorWriter to be codec agnostic#701

Merged
emcfarlane merged 5 commits intomainfrom
ed/fixEWCodecs
Mar 12, 2024
Merged

Fix ErrorWriter to be codec agnostic#701
emcfarlane merged 5 commits intomainfrom
ed/fixEWCodecs

Conversation

@emcfarlane
Copy link
Contributor

@emcfarlane emcfarlane commented Feb 23, 2024

This PR changes the ErrorWriter to be more lenient with classifying protocols. Errors codecs are agnostic to the codec used. Therefore we avoid checking the codec in classifying the request. IsSupported will return true for an unknown codec which allows the server to encode a better error message to the client. If not supported a 415 error response could be used to match gRPC server like handling. If not supported and trying to write an error the ErrorWriter will default to connects unary encoding (consistent with current behaviour).

Fixes #689

@emcfarlane emcfarlane requested a review from jhump February 23, 2024 17:50
@emcfarlane emcfarlane self-assigned this Feb 23, 2024
Base automatically changed from ed/fixEWOpts to main March 6, 2024 02:49
This PR changes the ErrorWriter to be more lenient with classifying
protocols. Errors codecs are agnostic to the codec used. Therefore we
avoid checking the codec in classifying the request. IsSupported will
return true for an unknown codec which allows the server to encode a
better error message to the client. If not supported a 415 error response
could be used to match gRPC server like handling. If not supported and
trying to write an error the ErrorWriter will default to connects unary
encoding.
@emcfarlane emcfarlane merged commit 872a6fd into main Mar 12, 2024
@emcfarlane emcfarlane deleted the ed/fixEWCodecs branch March 12, 2024 21:41
@jhump jhump added the bug Something isn't working label Mar 20, 2024
@jhump jhump mentioned this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ErrorWriter.IsSupported is overly strict

3 participants