fix: keep watching when a file in the tree fails to compile - #6313
Conversation
The initial pass in `find_and_compile` propagated a compile error, so `prqlc watch` exited before reaching the watch loop whenever any .prql file in the tree was broken — the state watch mode exists to iterate out of. Report and continue instead, matching what the watch loop already does for errors that appear after startup.
prql-bot
left a comment
prql-bot
left a comment
There was a problem hiding this comment.
Self-review. The change does what it says — the initial pass no longer aborts, and the watcher comes up — but let _ignore = compile_path(...) drops more than compile errors, and the non-compile ones have nothing else printing them.
compile_path returns Err from three places: the compile-error branch, which prints the compiler's diagnostics itself before returning anyhow!("failed to compile"); jinja::pre_process(...)?; and fs::write(sql_path, ...)?. Only the first is self-reporting. So on a watched tree whose .sql files can't be written, the pre-change command exited 1 with Permission denied (os error 13), and the post-change one printed Compiling <path>, wrote nothing, said nothing, and went on to watch forever:
$ chmod 555 watched/ # read-only output dir
$ prqlc watch watched/
Compiling watched/good.prql
Watching path "watched/"That's a worse failure than the one being fixed: the user gets no error and no output, and no amount of editing changes either. The watch loop has the same let _ignore, so the same write failure after startup is equally silent — parity with it is the PR's own argument, so fixing one path and not the other just relocates the inconsistency.
Pushing the fix: both call sites go through one compile_path_reporting_errors helper that prints the error instead of discarding it, so the walk still continues past every failure but nothing disappears.
$ prqlc watch watched/
Compiling watched/good.prql
watched/good.prql: Permission denied (os error 13)
Compiling watched/bad.prql
Error:
╭─[ :1:15 ]
...
watched/bad.prql: failed to compile
Watching path "watched/"Also in the push: the test comment claimed the file names put the failing file ahead of the second good one, but WalkDir isn't constructed with sort_by_file_name, so entry order is whatever read_dir returns. The test is a valid guard in any order — the bad file is always walked, so the parent commit's ? panics the unwrap regardless — but the comment asserted a guarantee that isn't there. Reworded to say why there are two good files rather than claiming an order.
`let _ignore = compile_path(...)` dropped every error, but only the compile-error branch prints anything of its own — a failed `.sql` write or a Jinja pre-process error went entirely silent, so a read-only output directory left the watcher running and producing nothing. Both call sites now go through one helper that prints the error instead of discarding it, keeping the walk going past every failure without losing the failures themselves.
prql-bot
left a comment
prql-bot
left a comment
There was a problem hiding this comment.
Self-review of the reporting commit. The behavior checks out — running the built binary against a read-only output directory prints watched/good.prql: Permission denied (os error 13), then the compile diagnostic for the broken file, then Watching path — but the CHANGELOG entry and the PR description still describe only the first half of the PR.
The entry says the initial pass now behaves "as it already did for errors that appear after it starts". The loop did keep watching, but it discarded every error (let _ignore = compile_path(...)), so a failure that produces no compiler diagnostic — an unwritable .sql output path, a Jinja pre-processing failure — was silent there too. Ending that silence is user-facing in its own right, and the contributing guide asks for a line per user-facing change ("If a change is user-facing, please add a line in CHANGELOG.md", web/book/src/project/contributing/development.md under Commits), so it belongs in the entry rather than being implied away by the initial-pass wording.
Pushing the reworded entry and a description that covers both halves; no code change.
# Conflicts: # CHANGELOG.md
2 participants
prqlc watch <dir>exited with status 1 instead of watching, whenever any.prqlfile under the watched tree failed to compile — which is the ordinary starting state for watch mode, and the one it exists to iterate out of.Note that "Watching path" is never printed: the initial pass in
find_and_compilepropagated the first compile error out throughrun, so the command returned beforewatch_and_compilewas ever called. Fixing the file does nothing, because nothing is watching it.The watch loop itself has always kept going past a file that fails — but by discarding the error outright (
let _ignore = compile_path(...)).compile_pathprints the compiler's own diagnostics before returning, so a compile failure was still visible there; the errors it returns without printing were not. An unwritable.sqloutput path, or a Jinja pre-processing failure, left the watcher running and producing nothing, with no indication why.Both call sites now go through one
compile_path_reporting_errors, which prints<path>: <error>and carries on, so the walk still continues past every failure but nothing disappears:The path prefix also supplies the file name the compiler's diagnostic omits — note the empty span header above.
Errors from the directory walk itself (
entry?) still abort, unchanged — an unreadable tree is not something watching can recover from.The regression test calls
find_and_compiledirectly over a temp dir holding two good files and one broken one, and asserts the two.sqloutputs exist and the walk returnedOk. It fails on the parent commit withcalled `Result::unwrap()` on an `Err` value: failed to compile. An end-to-end test of thewatchsubcommand isn't practical incli/test.rs, since the command blocks forever by design.