Skip to content

Update airlift to 258#22931

Merged
wendigo merged 1 commit intomasterfrom
serafin/airlift-257
Aug 5, 2024
Merged

Update airlift to 258#22931
wendigo merged 1 commit intomasterfrom
serafin/airlift-257

Conversation

@wendigo
Copy link
Copy Markdown
Contributor

@wendigo wendigo commented Aug 5, 2024

Changes requires as now, errors are rendered using jax-rs, rather than by the servlet container (Jetty).

See: https://eclipse-ee4j.github.io/jersey.github.io/apidocs/3.1.1/jersey/org/glassfish/jersey/server/ServerProperties.html#RESPONSE_SET_STATUS_OVER_SEND_ERROR

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 5, 2024

Could you confirm CI failures?

@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Aug 5, 2024

@ebyhr related. Fixing

@wendigo wendigo force-pushed the serafin/airlift-257 branch 2 times, most recently from 340a123 to 5d153cb Compare August 5, 2024 09:24
@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Aug 5, 2024

@ebyhr I've disabled the generation of error pages on the Jetty side. Now we are in full control over the entities that we return to the clients. This is for better alignment with HTTP/2

@wendigo wendigo force-pushed the serafin/airlift-257 branch 2 times, most recently from 30404db to 72e3b6a Compare August 5, 2024 11:00
@wendigo wendigo requested a review from losipiuk August 5, 2024 11:00
Copy link
Copy Markdown
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

% comment

Airlift disabled RESPONSE_SET_STATUS_OVER_SEND_ERROR in release 257.

sendError is problematic in HTTP/2 because it doesn't allow sending status
text along with the status code (there isn't such thing as status line in HTTP/2).
Other than that, the error mapping happened in two places: explicitly in the ThrowableMapper
and implictly in Jetty where status code and lines were used to generate an error page.

After this change, the mapping happens in a single place which makes it explicit.
All internal errors are rendered in the body of the response.
@wendigo wendigo force-pushed the serafin/airlift-257 branch from 72e3b6a to e53250d Compare August 5, 2024 13:18
@wendigo wendigo changed the title Update airlift to 257 Update airlift to 258 Aug 5, 2024
@wendigo wendigo merged commit 74aad42 into master Aug 5, 2024
@wendigo wendigo deleted the serafin/airlift-257 branch August 5, 2024 14:23
@github-actions github-actions bot added this to the 454 milestone Aug 5, 2024
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