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

Only use http1.1 #189

Merged
merged 1 commit into from
Aug 21, 2024
Merged

Only use http1.1 #189

merged 1 commit into from
Aug 21, 2024

Conversation

pnwpedro
Copy link
Collaborator

Description

  • Force HTTPX Client to use HTTP 1.1
  • Do not automatically retry transport errors

Motivation and context

Regarding HTTP 1.1., the h2 library has a bug that mishandles the HTTP2 protocol for GOAWAY frames. When it receives a GOAWAY, in progress streams are not allowed to complete. This means any time our edge issues a GOAWAY an in progress request will throw even if Fauna handled it correctly.

The automatic retry was causing the client to resend the query. This is an unsafe retry because the client doesn't know whether Fauna ran the query.

How was the change tested?

  • Our edge issues a GOAWAY consistently at 10000 requests. We observed this behavior with HTTP/2 enabled and not when disabled.

Screenshots (if appropriate):

Change types

    • Bug fix (non-breaking change that fixes an issue)
    • New feature (non-breaking change that adds functionality)
    • Breaking change (backwards-incompatible fix or feature)

Checklist:

    • My code follows the code style of this project.
    • My change requires a change to Fauna documentation.
    • My change requires a change to the README, and I have updated it accordingly.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pnwpedro pnwpedro merged commit dbcdd48 into main Aug 21, 2024
6 checks passed
@pnwpedro pnwpedro deleted the only-http1.1 branch August 21, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants