-
Notifications
You must be signed in to change notification settings - Fork 524
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
Cannot read properties of null (reading 'ref') using fetch with HTTP/2 enabled and assertion error #3011
Comments
this is the continuation of #2808 (comment) with @metcoder95 (if u want to try again) |
I'll try to simulate it based on your network conditions, this stills reproducible on your side, right? |
But i don't think is related to network. i was able to reproduce the same error also on https://replit.com/@gava97/UNdici-http2-test, you have to just wait that this happens, idk from 5 minutes to 1 hour. i was further checking (not tested yet) before that PR, the assert(client[kPending] === 0) was inside if (client.destroyed) but i don't know if it's the correct behaviour or is better to fix the "Cannot read properties of null (reading 'ref')" before EDITOn my side i can confirm that wrapping assert(client[kPending] === 0) inside an if statement if (client.destroyed) like it was before @ronag PR #2878 makes the code work ("Cannot read properties of null (reading 'ref')" still persist obviously but at least the script doesn't break brutally, for this i don't know really what can be the fix) Example: session.on('close', function () {
const { [kClient]: client } = this
console.log(Date.now(), "close", "client[kQueue].length", client[kQueue].length);
const err = this[kError] || new SocketError('closed', util.getSocketInfo(this))
client[kSocket] = null
//if (client[kPending] > 0) console.log(Date.now(), "close", "client.destroyed", client.destroyed);
if (client.destroyed) assert(client[kPending] === 0)
// Fail entire queue.
const requests = client[kQueue].splice(client[kRunningIdx])
console.log(Date.now(), "close", "requests.length", requests.length, "client.destroyed", client.destroyed);
for (let i = 0; i < requests.length; i++) {
const request = requests[i]
errorRequest(client, request, err)
}
client[kPendingIdx] = client[kRunningIdx]
assert(client[kRunning] === 0)
client.emit('disconnect', client[kUrl], [client], err)
client[kResume]()
}) |
@metcoder95 have you tried? any news? |
Yeah, I see what the issue is now; its in the way we handle the I'm working on a fix, will try to have something soon |
@metcoder95 tested, seems to work pretty good now, many thanks. |
Closed by #3057 |
Bug Description
While making continuous HTTP/2 requests after certain amount of time (minutes), happens that session goaway event is triggered, but meanwhile the close event is not fired and when a new request is pushed this error is thrown
Details
Reproducible By
https://replit.com/@gava97/UNdici-http2-test
(also run the test for minutes to replit.com throws the error)
Expected Behavior
No errors thrown or at least no assertion
Logs & Screenshots
Environment
Windows 11
Nodejs v20.12.0
undici 6.10.2
Additional context
I put some logs in the h2-client.js
a goaway is fired
but before the session fires close event a new request is pushed but the client[kHTTP2Session] property is null (set to null on goaway)
then the script goes to catch block
then when close finally fires
assetion error is thrown because client[kPending] is not 0, there still left requests in the queue
The text was updated successfully, but these errors were encountered: