http2: emit response event instead of trailers event

[MoonBall]

Fix #41251.

Should emit response event if cat == NGHTTP2_HCAT_HEADERS and status >= 200.

[nodejs-github-bot]

Review requested:

• @nodejs/http2
• @nodejs/net

[apapirovski]

Very nice find. And actually makes sense after reading the nghttp2 docs https://nghttp2.org/documentation/enums.html#c.nghttp2_headers_category

[szmarczak]

The naming is very confusing. HTTP/2 states that those are trailers (since HEADERS are sent with an END_STREAM flag), nghttp2 defines NGHTTP2_HCAT_RESPONSE (informational) + NGHTTP2_HCAT_HEADERS (final) and Node.js would emit response (however it also has a trailers event).

[mcollina]

lgtm

[mcollina]

The naming is very confusing. HTTP/2 states that those are trailers (since HEADERS are sent with an END_STREAM flag), nghttp2 defines NGHTTP2_HCAT_RESPONSE (informational) + NGHTTP2_HCAT_HEADERS (final) and Node.js would emit response (however it also has a trailers event).

Is this an objection? How would you like this PR to be changed?

[szmarczak]

In the Examples section of RFC7540:

     HTTP/1.1 100 Continue            HEADERS
     Extension-Field: bar       ==>     - END_STREAM
                                        + END_HEADERS
                                          :status = 100
                                          extension-field = bar

     HTTP/1.1 200 OK                  HEADERS
     Content-Type: image/jpeg   ==>     - END_STREAM
     Transfer-Encoding: chunked         + END_HEADERS
     Trailer: Foo                         :status = 200
                                          content-length = 123
     123                                  content-type = image/jpeg
     {binary data}                        trailer = Foo
     0
     Foo: bar                         DATA
                                        - END_STREAM
                                      {binary data}

                                      HEADERS
                                        + END_STREAM
                                        + END_HEADERS
                                          foo = bar

in this instance there are three HEADERS frames. I think it would make sense to have an information event, like in HTTP/1 API. What do you think?

[aduh95]

This needs a rebase.

[aduh95]

This still needs a rebase :)

[aduh95]

Superseded by #41739