-
-
Notifications
You must be signed in to change notification settings - Fork 944
pre-commit treats hook commands dying due to POSIX signal as successful #2970
Copy link
Copy link
Closed
Labels
Description
search you tried in the issue tracker
signal
describe your issue
When a hook command dies due to an unhandled signal, pre-commit treats the hook as successful.
You can see this for example with a node process that crashes due to hitting a memory limit (it signals itself with SIGTRACE):
$ pre-commit run --all-files --verbose node-crash
node-crash...............................................................
Passed
- hook id: node-crash
- duration: 0.02s
<--- Last few GCs --->
<--- JS stacktrace --->
#
# Fatal javascript OOM in GC during deserialization
#
$ echo $?
0
I've also included a simple kill-with-term hook in the attached config that reproduces it using only bash.
This happens because CompletedProcess.returncode will be a negative integer (e.g. -15 for SIGTERM) but pre-commit's returncode merging uses max() to collate the returncodes from the processes it spawns:
pre-commit/pre_commit/xargs.py
Lines 172 to 173 in a1f1d19
| for proc_retcode, proc_out, _ in results: | |
| retcode = max(retcode, proc_retcode) |
A draft fix like this resolves the issue for me and correctly shows the hooks as failing:
--- venv/lib/python3.10/site-packages/pre_commit/xargs.py.orig 2023-08-21 16:19:25.351609037 -0700
+++ venv/lib/python3.10/site-packages/pre_commit/xargs.py 2023-08-21 16:19:31.183503472 -0700
@@ -170,7 +170,7 @@
results = thread_map(run_cmd_partition, partitions)
for proc_retcode, proc_out, _ in results:
- retcode = max(retcode, proc_retcode)
+ retcode = retcode or proc_retcode
stdout += proc_out
return retcode, stdoutpre-commit --version
pre-commit 3.3.3
.pre-commit-config.yaml
repos:
- repo: local
hooks:
- id: kill-with-term
name: kill-with-term
entry: bash -c 'kill -TERM $$; while :; do :; done'
language: system
always_run: true
- id: node-crash
name: node-crash
entry: node --max-old-space-size=1 -p "console.log('ohai')"
language: system
always_run: true~/.cache/pre-commit/pre-commit.log (if present)
No response
Reactions are currently unavailable