Skip to content

hook: Fix custom hook errors not propagating correctly

Patrick Steinhardt requested to merge pks-hooks-custom-errors into master

In commit c6821337 (updateref: Generalize PreReceiveError according to its usage, 2021-08-25), we have changed how pre-receive errors get propagated to callers, where it's now instead using a more generalized HookError. Part of this change was a refactoring of the error message to be more descriptive: instead of printing hook errors printed to their stdout/stderr as-is, we prepend the actual error we have been seeing to pinpoint why hook execution failed.

As it turns out though, Rails requires the messages printed by custom hooks to be returned as-is. This is to support a feature where hooks can print messages either with a "GitLab:" or with a "GL-HOOK-ERR:" prefix, which indicates to Rails that it should present the error message to the user. Prepending the error message with unrelated metadata obviously breaks this feature.

We are in the process of fixing the whole architecture to use structured error messages instead, which contain error information in a more detailed way and don't rely on parsing and/or prefixing error messages. This doesn't work easily with custom hooks though: we have no control of stdin and stderr of the command given that these may be connected to git-receive-pack(1), which would in turn stream anything printed there to the user. So we have no easy way of inspecting the error messages and thus cannot properly bubble them up in a custom error type, either. While we could introduce e.g. a io.TeeReader to enable for this, this introduces additional complexity that may not be worth the hassle given we're slowly moving towards a deprecation of custom hooks in the first place.

So let's mark custom hook errors as such and add some special handling in our hook updater so that both stderr and stdout are returned as-is.

Changelog: fixed

Fixes gitlab#342607 (closed)

Fixes gitlab#342122 (closed)

Merge request reports