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

h2: If we send a GOAWAY, we shouldn't be surprised at a connection reset #119

Closed
fasterthanlime opened this issue Oct 16, 2023 · 0 comments · Fixed by #139
Closed

h2: If we send a GOAWAY, we shouldn't be surprised at a connection reset #119

fasterthanlime opened this issue Oct 16, 2023 · 0 comments · Fixed by #139
Labels
bug Something isn't working

Comments

@fasterthanlime
Copy link
Collaborator

Describe the bug

Not really a bug but see this log, that's the end of http2/5.1.2:

 WARN fluke::h2::server: connection error: max concurrent streams exceeded (more than 32) (MaxConcurrentStreamsExceeded { max_concurrent_streams: 32 }) (code ProtocolError)
DEBUG fluke::h2::server: Sending GoAway last_stream_id=65 error_code=ProtocolError
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 29, payload: BodyChunk }
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 63, payload: Headers }
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 31, payload: BodyChunk }
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
        ✔ 1: Sends HEADERS frames that causes their advertised concurrent stream limit to be exceeded
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 33, payload: BodyChunk }

Finished in 0.0596 seconds
1 tests, 1 passed, 0 skipped, 0 failed
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 35, payload: BodyChunk }
DEBUG fluke::h2::server: Handler completed successfully, gave us a responder
DEBUG fluke::h2::server: Writing ev=H2Event { stream_id: 37, payload: BodyChunk }
DEBUG fluke::h2::server: caught error from one of the tasks: read error: read_into for read_and_parse::<fluke::h2::parse::Frame> / Read(
    Error {
        msg: "read_into for read_and_parse::<fluke::h2::parse::Frame>",
        source: Os {
            code: 104,
            kind: ConnectionReset,
            message: "Connection reset by peer",
        },
    },
)
ERROR fluke_h2spec: error serving client 127.0.0.1:60488: read error: read_into for read_and_parse::<fluke::h2::parse::Frame>
DEBUG fluke::h2::server: Handler returned an error: could not send event to h2 connection handler
DEBUG fluke::h2::server: Handler returned an error: could not send event to h2 connection handler
 WARN fluke::h2::encode: could not send event to h2 connection handler
 WARN fluke::h2::encode: could not send event to h2 connection handler

Expected behavior

If we've sent a GOAWAY, we should fully expect to 1) not be able to write further messages, 2) get a connection reset when we try to read more messages.

This can probably be done by just having a "has_sent_goaway" flag somewhere.

@fasterthanlime fasterthanlime added the bug Something isn't working label Oct 16, 2023
@fasterthanlime fasterthanlime changed the title If the h2 server sends GOAWAY it should expect a connection reset h2: If we send a GOAWAY, we shouldn't be surprised at a connection reset Oct 17, 2023
fasterthanlime added a commit that referenced this issue Jan 25, 2024
fasterthanlime added a commit that referenced this issue Jan 25, 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 a pull request may close this issue.

1 participant