Skip to content

fix: report CLI read/write errors instead of crashing - #895

Open
troclaux wants to merge 1 commit into
NaturalIntelligence:masterfrom
troclaux:fix/cli-async-error-handling
Open

troclaux wants to merge 1 commit into
NaturalIntelligence:masterfrom
troclaux:fix/cli-async-error-handling

Conversation

@troclaux

Copy link
Copy Markdown

Purpose / Goal

Fixes #894.

The built-in CLI crashed with an uncaught exception when the input file was missing or the -o output file couldn't be written. The try/catch in src/cli/cli.js was meant to print Seems an invalid file or stream., but the throw err calls run inside async callbacks (fs.readFile, fs.writeFile, ReadToEnd.readToEnd). Those run after the try block has already returned, so the catch never sees them.

Input

node src/cli/cli.js /nonexistent/missing.xml

Actual output (before): uncaught ENOENT, with a stack trace pointing at throw err; (cli.js:80).

Expected output (after)

Seems an invalid file or stream.Error: ENOENT: no such file or directory, open '/nonexistent/missing.xml'

The exit code is 1.

Changes

  • The three callbacks now report the error through a small onError helper and return, instead of throwing. The helper prints the existing message and sets process.exitCode = 1. The existing catch uses the same helper, so it now also exits non-zero.
  • Added spec/cli_spec.js with one positive case (parse from stdin) and two negative cases (missing input file, unwritable -o path). Both negative cases fail before this change and pass after it.

I reproduced the missing-file and unwritable-output cases. I couldn't trigger the stdin stream-error path from the shell, but it had the same throw-in-callback shape, so it gets the same handling.

Testing

  • npm test: 335 specs, 0 failures.
  • npm run test-types: passes.
  • Benchmarks: not run. This change only touches src/cli/cli.js, which the parser and builder don't import, so benchmark/XmlParser.mjs and benchmark/XmlBuilder.mjs are unaffected.

Type

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

This branch has not been deployed

No deployments
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.

CLI crashes with an uncaught exception when the input file is missing

1 participant