feat: show warning on 431 response

[sapphi-red]

Description

Show warnings when HPE_HEADER_OVERFLOW error happened. Currently the server does not output any errors or warnings.

close #547

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[bluwy]

I think it's a bit specific that we only handle this specific error, maybe we should log the err generically instead? (not sure if that provides any message).

[sapphi-red]

clientError event seems to be called with 400 and 431 (this one) but I'm not sure if we should output a warning for 400 too.

The err looks like this if it is output directly.

Maybe something like this is better?

Suggested change

	server.on('clientError', (err, socket) => {
	if ((err as any).code === 'HPE_HEADER_OVERFLOW') {
	if (!socket.writableEnded) {
	socket.end(
	'HTTP/1.1 431 Request Header Fields Too Large\r\n\r\n' +
	'Request Header was too large. Node.js limits request header size. ' +
	'Use https://nodejs.org/api/cli.html#--max-http-header-sizesize to change max header size.'
	)
	}
	logger.warn(
	colors.yellow(
	'Server / WS server responded with "431 Request Header Fields Too Large." ' +
	'Node.js limits request header size and the request was dropped. ' +
	'Use https://nodejs.org/api/cli.html#--max-http-header-sizesize to change max header size.'
	)
	)
	}
	})
	server.on('clientError', (err, socket) => {
	const code = (err as any).code
	if (code === 'HPE_HEADER_OVERFLOW') {
	logger.warn(
	colors.yellow(
	'Server / WS server responded with an error. ' +
	`${err.message} (code: ${code})`
	)
	)
	}
	})

[patak-dev]

I think the custom error message here is useful. Maybe something in between this two?
Another option is now that you started a troubleshooting page, we could start adding these longer explanations there, linking to it from the error instead of needing these messages that with time could add up to the size in Vite.

[bluwy]

Thanks for the explanation @sapphi-red. I think the original PR makes sense then given two errors only happen here. It's strange that Node gives a cryptic error though, and my concern is with patak too that this increases the size for a very specific case.

I like the idea of putting this in the troubleshooting page too, we could probably elaborate more there too.

[sapphi-red]

Sounds nice. I've move the details to troubleshooting page 👍

[patak-dev]

Maybe we could discuss a bit in today's meeting. We could start moving other messages to the new guide, and add a section in the CONTRIBUTING guide to guide other contributors to do the same from now on. Maybe these should be in a section that only list errors, like https://yarnpkg.com/advanced/error-codes

[haoqunjiang]

FYI, how React team built their error code system: https://reactjs.org/blog/2016/07/11/introducing-reacts-error-code-system.html

[liuweifeng]

hi @sapphi-red ,in the above code, the socket is not closed, in my case, it will hang up the webpage untill timeout reached. Could you please help me review this pull request? #9816