Skip to content

Propagate JSON parse errors#326

Open
StantonMatt wants to merge 1 commit into
extrabacon:masterfrom
StantonMatt:fix-json-parse-error-propagation
Open

Propagate JSON parse errors#326
StantonMatt wants to merge 1 commit into
extrabacon:masterfrom
StantonMatt:fix-json-parse-error-propagation

Conversation

@StantonMatt
Copy link
Copy Markdown

Summary

  • catch exceptions from stdout parser functions before emitting message events
  • emit a parseError event and pass the first parse failure to .end() / PythonShell.run() so callers can handle invalid JSON output
  • keep user message listener exceptions outside the parser catch path so normal EventEmitter behavior is preserved

Fixes #253.

Validation

  • npm test -- --grep "parseError|JSON output"
  • npm test
  • git diff --check
  • review-fix-loop clean after addressing one reviewer finding

@kikiminyes
Copy link
Copy Markdown

I checked the core path locally.

  • npm run compileOnce passes.
  • I could not run the Python-dependent suite in this Windows environment because Python spawning fails before the tests run, so I validated the changed JS path with a mocked child process instead.
  • With invalid JSON on stdout, parseError is emitted, .end() receives PythonShellParseError, and PythonShell.run(..., { mode: "json" }) rejects with the same parse error and empty logs.

The implementation looks directionally correct to me. Keeping parse failures on a separate parseError event avoids Node's special unhandled error event behavior while still surfacing the failure through the completion path.

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.

Can't catch JSON.parse() error?

2 participants