Propagate JSON parse errors - #326
Conversation
|
I checked the core path locally.
The implementation looks directionally correct to me. Keeping parse failures on a separate |
PNHD
left a comment
There was a problem hiding this comment.
The parser/emit ordering on this head is sound, and the focused/full npm suites plus repository Prettier check pass locally on an LF-safe checkout. One required piece is still missing before this is ready as a library API change: the PR exports PythonShellParseError and adds a public parseError event, but README's event documentation does not mention either. For direct PythonShell callers without an .end() callback, parseError is the parser-failure surface; leaving it undocumented can turn the old crash into a failure users do not know how to observe. Please document the new event/class, when it fires, and how .end() / PythonShell.run() receive the first parse failure. I have an overlapping #334 with a different API shape and will defer to maintainer preference rather than requiring either architecture here.
Summary
messageeventsparseErrorevent and pass the first parse failure to.end()/PythonShell.run()so callers can handle invalid JSON outputmessagelistener exceptions outside the parser catch path so normal EventEmitter behavior is preservedFixes #253.
Validation
npm test -- --grep "parseError|JSON output"npm testgit diff --checkreview-fix-loopclean after addressing one reviewer finding