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

Use authentication error message from response body #15720

Merged
merged 1 commit into from
Jan 17, 2023

Conversation

electrum
Copy link
Member

@electrum electrum commented Jan 14, 2023

Description

All versions of Trino (since c0bcc9b) send the error message in the response body. The reason phrase does not exist in HTTP/2.

Release notes

(x) Release notes are required, with the following suggested text:

# CLI
* Improve authentication error messages when accessing the server over HTTP/2. ({issue}`15720`)

# JDBC driver
* Improve authentication error messages when accessing the server over HTTP/2. ({issue}`15720`)

@cla-bot cla-bot bot added the cla-signed label Jan 14, 2023
@electrum electrum requested a review from dain January 14, 2023 07:03
All versions of Trino send the error message in the response body.
The reason phrase does not exist in HTTP/2.
@electrum
Copy link
Member Author

This is required by #15703 since the Jetty 10 server no longer sends the provided reason phrase (at least via the servlet APIs used by our version of Jersey).

@electrum electrum merged commit d5c6424 into trinodb:master Jan 17, 2023
@electrum electrum deleted the auth-error branch January 17, 2023 20:56
@github-actions github-actions bot added this to the 406 milestone Jan 17, 2023
@colebow
Copy link
Member

colebow commented Jan 18, 2023

I think we can skip release notes for this. It's generally been the practice that if users will encounter better messaging, that messaging is self-documenting when it's encountered, and it doesn't really do them much good to know that the messaging will be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants