Uncaught AssertionError thrown due to a possible race condition?

[kartikk221]

Bug Description

I run an application which makes a lot of requests per second using HTTP/2 over a fixed pool of about 500 Dispatchers.

Everything works great but sometimes when the upstream tends to have a higher error rate, these errors come in the form of a weird assertion error within our logs as uncaught promise rejections.

Reproducible By

I am not sure how I could provide a reproducible example for this as it does happen randomly and is certainly tied to upstream request failures.

Expected Behavior

The request failure should be properly thrown and caught by the request itself instead of the global unhandled rejection handler within our application.

Logs & Screenshots

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
    
      assert(this.callback)
    
        at RequestHandler.onConnect (/root/cluster-worker/node_modules/undici/lib/api/api-request.js:82:5)
        at Request.onConnect (/root/cluster-worker/node_modules/undici/lib/core/request.js:228:29)
        at writeH2 (/root/cluster-worker/node_modules/undici/lib/dispatcher/client-h2.js:345:13)
        at Object.write (/root/cluster-worker/node_modules/undici/lib/dispatcher/client-h2.js:119:14)
        at _resume (/root/cluster-worker/node_modules/undici/lib/dispatcher/client.js:601:50)
        at resume (/root/cluster-worker/node_modules/undici/lib/dispatcher/client.js:523:3)
        at Client.<computed> (/root/cluster-worker/node_modules/undici/lib/dispatcher/client.js:250:31)
        at ClientHttp2Stream.<anonymous> (/root/cluster-worker/node_modules/undici/lib/dispatcher/client-h2.js:531:22)
        at Object.onceWrapper (node:events:632:28)
        at ClientHttp2Stream.emit (node:events:530:35)

Environment

Distributor ID: Ubuntu
Description: Ubuntu 24.04.1 LTS
Release: 24.04
Codename: noble
Node: v20.18.2

Additional context

I have looked through the source code and may have a clue as to what is potentially causing this.

The error is thrown in this snippet:

undici/lib/api/api-request.js

Lines 76 to 86 in c7f3d77

	onConnect (abort, context) {
	if (this.reason) {
	abort(this.reason)
	return
	}
	assert(this.callback)
	this.abort = abort
	this.context = context
	}

As we can observe, the culprit is that the assert(this.callback) fails thus something is nullifying the callback before the onConnect is fired.

There are only two places in the rest of this file where this is done which is within the onHeader() and onError() methods. The onHeader() is likely not the culprit as well the onConnect will always fire before the onHeader due to the request lifecycle.

The onError() thus remains to be the only culprit which can be causing this where the onError is for some reason firing before the onConnect().

In fact, there seems to be a TODO: already there which potentially questions this problematic this.callback = null operation on line 146:

undici/lib/api/api-request.js

Lines 141 to 150 in c7f3d77

	onError (err) {
	const { res, callback, body, opaque } = this
	if (callback) {
	// TODO: Does this need queueMicrotask?
	this.callback = null
	queueMicrotask(() => {
	this.runInAsyncScope(callback, null, err, { opaque })
	})
	}

[metcoder95]

Great findings! Tho, I doubt the issue roots there; what you point out seems mostly the effect of another issue.

It is possible that, when error rates arise there might be some race condition that is not cleaning the resources when it should, and attempting to read the server response when it is not possible to be read.

Can you try to build an Minimum Reproducible Example or elaborate more on the errors the server returns that often leads to this?