Skip to content

fix(server): fail fast with a clear message on unsupported Node.js versions#6584

Open
gaurav0107 wants to merge 3 commits into
FlowiseAI:mainfrom
gaurav0107:fix/5670-starting-a-local-server-fails
Open

fix(server): fail fast with a clear message on unsupported Node.js versions#6584
gaurav0107 wants to merge 3 commits into
FlowiseAI:mainfrom
gaurav0107:fix/5670-starting-a-local-server-fails

Conversation

@gaurav0107

Copy link
Copy Markdown

Summary

Running npx flowise start on an unsupported Node.js version crashes at startup with an opaque error:

ReferenceError: File is not defined
    at .../cheerio/node_modules/undici/lib/web/webidl/index.js:533:48

The File global only exists on Node.js >= 20, and undici (pulled in transitively via cheerio, used by the Cheerio document loader and the Web Scraper tool) references it at module-load time. Flowise declares engines.node: "^24", but that field is only advisory — npm install -g flowise succeeds on older runtimes and the user only finds out via the cryptic crash, with no hint that the real cause is their Node version. This matches the diagnosis in the issue thread ("are you on version 18+?").

This PR adds a small, dependency-free version guard that runs at the very top of bin/run, before @oclif/core loads any command (the start command imports the server — and therefore cheerio — at module scope, so the check has to happen first). When the running Node.js major is below the one declared in engines.node, it prints an actionable message and exits with code 1:

Flowise requires Node.js >= 24. You are running v18.20.0. Please upgrade Node.js
(https://nodejs.org) and try again. Running on an unsupported version causes startup
errors such as "ReferenceError: File is not defined".

Design notes:

  • The required major is read from this package's own engines.node, so it stays in sync automatically.
  • The comparison fails open: an unparseable running-version string is treated as supported, so an unexpected version format never blocks startup.
  • The guard in bin/run is wrapped in try/catch, so the check can never itself break startup (e.g. when dist/ is not built in a dev tree).
  • The logic lives in a pure, unit-tested helper (src/utils/nodeVersion.ts); bin/run only wires it in.

It does not attempt to make Flowise run on an unsupported Node.js version (not possible — undici needs the platform File global); it surfaces the real requirement instead of a stack trace.

Verification

  • pnpm --filter flowise test — new suite src/utils/nodeVersion.test.ts passes (13/13): version parsing, engines range parsing, support comparison (including fail-open), the user-facing message, and assertNodeVersion.
  • tsc (server) — clean, no new type errors introduced.
  • eslint + prettier --check — clean on all changed files.
  • Manual: requiring the compiled helper and calling assertNodeVersion('v18.0.0') returns the error message and process.exit(1) path; v24+ returns { ok: true }.

Scope is limited to packages/server (bin/run + one new helper + its test); no proprietary paths are touched.

Closes #5670

…rsions

Running `flowise start` on Node < 20 crashes at startup with an opaque
`ReferenceError: File is not defined`, thrown from cheerio's transitive
undici when it references the `File` global (added in Node 20). Flowise
declares `engines.node: ^24`, but that field is only advisory, so users on
older runtimes hit the cryptic crash with no hint of the real cause.

Add a small, dependency-free version guard that runs at the very top of
bin/run (before @oclif/core loads any command) and prints an actionable
message naming the required and current Node versions, then exits 1. The
guard reads the required major from engines.node, fails open on unparseable
input, and is wrapped so it can never itself block startup. The logic lives
in a pure, unit-tested helper.

Closes FlowiseAI#5670

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Node.js version check at startup to fail fast with a clear error message if the running Node.js version is unsupported (specifically to prevent opaque crashes on Node < 20 where the File global is missing). It adds utility functions for parsing and asserting the Node.js version along with corresponding unit tests. The review feedback suggests simplifying redundant string conversions in parseMajor, using loose equality (== null) for nullish checks in isSupportedNodeVersion, and removing unnecessary imports from @jest/globals in the test file to avoid potential dependency resolution issues.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

*/
export const isSupportedNodeVersion = (current: string, requiredMajor: number): boolean => {
const major = parseMajor(current)
if (major === null) return true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the repository's standard idioms, loose equality (== null) should be used for nullish checks to cover both null and undefined. Additionally, ensure that the check returns false if the node's version is missing to prevent synchronization UI elements from appearing for nodes that do not support versioning.

Suggested change
if (major === null) return true
if (major == null) return false
References
  1. In JavaScript/TypeScript, use loose equality (== null) as a standard idiom for a 'nullish' check that covers both null and undefined.
  2. When implementing version comparison for nodes, ensure that the check returns false if the node's version is missing. This prevents synchronization UI elements from appearing for nodes that do not support versioning, maintaining consistency with legacy implementations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I intentionally kept this returning true here (and used strict === null): parseMajor returns number | null (never undefined), and the goal is to fail open — when the running Node version string can't be parsed (nightly/custom builds), treat it as supported rather than block startup. Returning false would fail closed and could block legitimate users on unusual-but-valid runtimes. Note this is the Node.js process runtime version, not component/node versioning, so the legacy-versioning guidance doesn't apply.

Comment on lines +16 to +19
export const parseMajor = (version: string): number | null => {
const match = /^v?(\d+)\./.exec(String(version ?? '').trim())
return match ? parseInt(match[1], 10) : null
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since the version parameter is already typed as a string, the nullish coalescing and String() conversion are redundant. We can simplify this to directly trim the string.

Suggested change
export const parseMajor = (version: string): number | null => {
const match = /^v?(\d+)\./.exec(String(version ?? '').trim())
return match ? parseInt(match[1], 10) : null
}
export const parseMajor = (version: string): number | null => {
const match = /^v?(\\d+)\\./.exec(version.trim())
return match ? parseInt(match[1], 10) : null
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — simplified to version.trim() in ec6e557. Thanks.

@@ -0,0 +1,78 @@
import { describe, expect, it } from '@jest/globals'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Importing from @jest/globals is unnecessary because Jest globals (describe, expect, it) are already available globally and typed via @types/jest. Removing this import avoids potential phantom dependency resolution issues in strict package managers like pnpm.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ec6e557 — dropped the @jest/globals import to rely on Jest's global describe/expect/it (typed via @types/jest), matching the majority of the server test suite. Thanks.

Drop the unused `err` binding from the bin/run try/catch.
- parseMajor: drop the redundant String()/nullish coalescing now that the
  parameter is typed as string
- nodeVersion.test.ts: rely on Jest's global describe/expect/it (typed via
  @types/jest) instead of importing from @jest/globals, matching the
  majority convention of the server test suite
@gaurav0107 gaurav0107 marked this pull request as ready for review June 30, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Starting a local server fails

1 participant