Skip to content

fix: Fixed the issue that there is no running request when http2 goaway#3875

Merged
metcoder95 merged 1 commit intonodejs:mainfrom
ShenHongFei:fix-http2-goaway
Nov 24, 2024
Merged

fix: Fixed the issue that there is no running request when http2 goaway#3875
metcoder95 merged 1 commit intonodejs:mainfrom
ShenHongFei:fix-http2-goaway

Conversation

@ShenHongFei
Copy link
Contributor

This relates to...

onHttp2SessionGoAway throws an error

TypeError: Cannot read properties of undefined (reading 'onError')
    at Object.errorRequest (node:internal/deps/undici/undici:3935:13)
    at ClientHttp2Session.onHttp2SessionGoAway (node:internal/deps/undici/undici:5969:8)
    at ClientHttp2Session.emit (node:events:513:28)
    at ClientHttp2Session.emit (node:domain:489:12)
    at Http2Session.onGoawayData (node:internal/http2/core:683:11)
    at Http2Session.callbackTrampoline (node:internal/async_hooks:130:17)

The reason is const request = client[kQueue][client[kRunningIdx]] where request is empty.
Then I tried adding some console.log

function onHttp2SessionGoAway (errorCode) {
  ...

  // Fail head of pipeline.
  console.log('clinet[kRunningIdx]:', clinet[kRunningIdx])  // clinet[kRunningIdx]: 2
  console.log('clinet[kPendingIdx]:', clinet[kPendingIdx])  // clinet[kPendingIdx]: 2
  console.log('clinet[kQueue].length:', clinet[kQueue].length)  // clinet[kQueue].length: 2
  console.log('clinet[kQueue]:', clinet[kQueue])  // clinet[kQueue]: [null, null]
  
  const request = client[kQueue][client[kRunningIdx]]
  client[kQueue][client[kRunningIdx]++] = null
  util.errorRequest(client, request, err)  // throws "Cannot read properties of undefined"
  client[kPendingIdx] = client[kRunningIdx]

  assert(client[kRunning] === 0)

  ...
}

Rationale

fix this issue

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@metcoder95 metcoder95 changed the title Fixed the issue that there is no running request when http2 goaway fix: Fixed the issue that there is no running request when http2 goaway Nov 24, 2024
@metcoder95
Copy link
Member

Can you add a test for it?

@ShenHongFei
Copy link
Contributor Author

I don't know how to construct a server to reproduce the http2 goaway situation here, and it's not easy to write a test case

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants