Custom hook errors don't display in merge request widget

Per https://docs.gitlab.com/ee/administration/custom_hooks.html#custom-error-messages, we should show error messages from custom hooks in the merge request widget. When I tested this, the widget simply spins on 'Merge in progress' forever.

Of note, a custom hook error is displayed if you attempt to commit in the web UI and something goes wrong. Somewhere along the way, we accidentally broke this functionality in the MR widget, though.

Steps to reproduce:

1. Create a project and make sure there is at least a master branch with some files.
2. Add a custom hook on the filesystem. It's contents can be simple. See https://docs.gitlab.com/ee/administration/custom_hooks.html#setup for details about location and permissions. When I tested it, I put:

     >&2 echo "error"
     exit 1

3. Attempt to create a new branch in the UI. The branch creation should fail with 'error'. This validates that your hook works.
4. Temporarily move the hook.
5. Create the branch since it won't fail this time.
6. Add a file or two to the new branch.
7. Create a merge request
8. Attempt to merge.
9. Observe that the error is not shown, but 'Merge in progress' spins forever. There may be an error in the Sidekiq log.

For reference, Zendesk issue https://gitlab.zendesk.com/agent/tickets/66129

changed milestone to gitlab-org/gitlab-ce%21

assigned to @psimyn

@dblessing This worked for me locally, do you mean here?

Also, should there be 'move hook back' between steps 7 & 8?

removed assignee

Perhaps it has been fixed in 8.17+, @psimyn ? We are having this issue in our 8.16.4 EE. I will let you know if upgrade fixes it.

I tested this on 8.17.0 yesterday. @psimyn Yes, you're right about missing a step there. I'll retest today on 8.17.1. I did not have CI enabled, either so I didn't have the 'merge when build succeeds' button. Maybe that's significant - maybe not.

We have upgraded to 8.17.2. And when "fast-forward merges" are enabled in the project, then it still doesn't work. But if you switch to default "merge commit" then the error from the git hook DOES show up!

So please update the desc that this happens when "fast-forward merges" are enabled, @dblessing .

@grzegorz-dubicki Thanks for this detail! I'll try to repro with that in mind.

I believe issue is in FfMergeService not raising Error, since the response to MergeRequestWidget doesn't have a merge_error

Relabelling as backend

@smcgivern might have some ideas for this

added gitlab-org/gitlab-ce901060 and removed gitlab-org/gitlab-ce127616 labels

Thanks @psimyn - that makes this an EE-only issue.

moved from gitlab-org/gitlab-ce#28806

moved from smcgivern/gitlab-ee#1

added ~874214 ~91797 ~155976 labels

changed milestone to %9.0

assigned to @smcgivern

created branch 1852-custom-hook-errors-don-t-display-in-merge-request-widget

mentioned in merge request !1375 (merged)

Thanks @dblessing @grzegorz-dubicki @psimyn! We did recently fix this for non-FF merges, so !1375 (merged) should take care of the remainder, and will be fixed in 9.0.

closed via merge request !1375 (merged)