Update npm run to fix hpe_header_overflow in recent nodejs versions

[guwenqing]

The change is needed to fix hpe_header_overflow in recent nodejs versions.

Nodejs has set max header size to 8k in http_parser from v10.14.2 in LTS versions,
need to run webpack devServer with a larger header size to make the proxy work.

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• New Query Runner (Data Source)
• New Alert Destination
• Other

Description

Nodejs has set max header size to 8k in http_parser from v10.14.2 in LTS versions,
need to run webpack devServer with a larger header size to make the proxy work.
Issue is reported to webpack and workaround has been presented here:
webpack/webpack-dev-server#1711

Not sure if existing dev documents should be updated to tell which version of nodejs should be used. The override as used in this change is only applicable to nodejs version higher than v10.15.0.

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[gabrieldutra]

@arikfr this seems a simple workaround for the issue with npm run start. I've tested it here, it works to me and also it doesn't seem to break the command for old Node versions (I've tested it with version 6). Do you have any other comments for it?

[arikfr]

Super! Thanks, @guwenqing :)

[arikfr]

I'm getting the following error:

node: bad option: --max-http-header-size=16385

Is it possible this option is available only in newer versions of Node? (I'm using v10)

[guwenqing]

According to what I read, it should only be used with later version. That is why I mentioned if we should modify the dev guide.
Not sure if a better workaround should be created to do differently based on different version.

[gabrieldutra]

I'm running it without issues in node v10.15.3. I also checked docker-compose run --rm server npm run start to see what would happen with an old version of node (v6.16.0), also no issues to run the command. In any case as this affects only npm run start and the command wasn't working before due to this proxy issue...

How about adding a note that npm run start is available for Node >v10.14.2 (maybe this is your issue @arikfr?) and for any version prior to it there's npm run watch?

[arikfr]

@gabrieldutra I wonder if the default should be support the new version? I mean, it's more likely that most people use an older version?

[gabrieldutra]

@gabrieldutra I wonder if the default should be support the new version? I mean, it's more likely that most people use an older version?

The Max Header issue seemed to be introduced in a secure release and to affect minors (I was thinking it was major related before 😬) from: 6.15, 8.14, 10.14 and 11.3. As it's a secure release, it should affect the majority of people at some point...

This explains why my test with v6.16.0 worked...

There's the option of a workaround like

"start": "node --max-http-header-size=16385 ./node_modules/webpack-dev-server/bin/webpack-dev-server.js || webpack-dev-server"

But there are issues with it...

[deecay]

How is the value 16385 chosen? I needed to increase this value to perform a fork.

[arikfr]

@gabrieldutra can't say I was much worried about security issues to upgrade local Node version... :)

@deecay is it consistent that on every fork request it fails with the 16385 value? If it is, can you try disabling chromelogger and check if it solves this?

You can disable it by changing this function:

redash/redash/handlers/chrome_logger.py

Lines 50 to 54 in 4508975

	def init_app(app):
	if not app.debug:
	return
	app.after_request(chrome_log)

[deecay]

@arikfr Query with 0 or 1 visualization are okay. Query with 2 or more visualization failed to fork.

If I disable chrome_logger, then the issue went away.