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

Fix ErrorWriter to be codec agnostic #701

Merged
merged 5 commits into from
Mar 12, 2024
Merged

Fix ErrorWriter to be codec agnostic #701

merged 5 commits into from
Mar 12, 2024

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

connect_ext_test.go Outdated Show resolved Hide resolved
@emcfarlane emcfarlane self-assigned this Feb 23, 2024
connect_ext_test.go Outdated Show resolved Hide resolved
error_writer.go Show resolved Hide resolved
error_writer.go Show resolved Hide resolved
error_writer.go Outdated Show resolved Hide resolved
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.
error_writer.go Outdated Show resolved Hide resolved
error_writer.go Outdated Show resolved Hide resolved
@emcfarlane emcfarlane merged commit 872a6fd into main Mar 12, 2024
12 checks passed
@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