dbt-labs / dbt-labs/dbt-autofix

[BUG] Pre-commit hook reports "Passed" when errors occur (silent failures)

Open
#301 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
88
Forks
19
Avg merge
1d 1h
Merged PRs (30d)
13

Description

The pre-commit hook reports "Passed" even when specific errors occur during execution. Errors are printed to console but don't cause the hook to fail with a non-zero exit code.

## Example of Current Behavior
[When running the pre-commit hook with `--verbose` on a project with no dbt_project.yml found:](https://github.com/dbt-labs/dbt-autofix/issues/244#issuecomment-3639394534)

Image

## Root Cause

In [src/dbt_autofix/refactor.py,](https://github.com/dbt-labs/dbt-autofix/blob/main/src/dbt_autofix/refactor.py) certain error conditions only print to console but don't call exit(1) or raise exceptions. The pre-commit hook ([pre_commit_hooks/check_deprecations.py](https://github.com/dbt-labs/dbt-autofix/blob/main/pre_commit_hooks/check_deprecations.py)) only returns non-zero exit codes when has_changes=True, not when errors occur. Therefore, it does correctly flag as "Failed" when files are changed as intended; however, it does not flag failures upstream that occur even before dbt-autofix can act on the project.

The main silent failures include:
- dbt_project.yml not found
- SQL file processing exceptions

These upstream failures only print an error message to stderr which is currently, not recognized by the pre-commit hook as an error since no exception or exit(1) is raised/called.

## Expected Behavior
For the above examples, pre-commit should return non-zero exit codes for these errors that don't necessarily involve file changes, should show "Failed" instead of "Passed", and in general, should have consistent error-handling.

## Proposed Solutions
So I think there are multiple directions this can go, but wanted to first get the POV from the maintainers about the intended approach to error handling:
- Should `refactor.py` raise exceptions for these upstream/critical errors or should it continue printing errors and we instead augment the pre-commit hook to somehow detect these printed errors?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.