search

mantisbt-plugins / source-integration

Source control integration plugin framework for MantisBT, including support for Github, Gitlab, Bitbucket, Gitea, Gitweb, Cgit, Subversion, Mercurial and more
http://noswap.com/projects/source-integration/
MIT License
181 stars 130 forks source link

POST to checkin.php always responses 200 despite any errors occuring. #266

Closed obmsch closed 1 month ago

obmsch commented 6 years ago

I am wondering if this is intentional behaviour to keep the "normal" caller happy or a just make it work situation. For now the only "reliable" option left to catch errors is to parse the actual response for any traces of errors or exceptions.

Background: I am running a post-commit hook (svn) to automatically checkin commits to MantisBT with a POST to checkin.php. So there I need a safe way to inform the client (exitcode, message, mail,...) if the checkin really happened.

dregad commented 6 years ago

I am wondering if this is intentional behaviour to keep the "normal" caller happy or a just make it work situation

Your guess is as good as mine :wink:... This code has not been touched since 2012, long before I took over.

It would probably make sense to return an HTTP 400 instead of relying on die() statements. I don't think there would be any problem doing that, but I don't know for sure (I only use the plugin with Github, which does not use this functionality).

obmsch commented 6 years ago

I'd say a 400 would be ok for the dies in checkin.php, but a 500 more appropiate if an error occurs in the 'source'-handler. I personally don't care much about the former (those only happen if the repo setup is wrong or there's a bug in the hook - that should be fixed before deployed the first time), the latter ones are the more interesting. Maybe there is a temp problem with the called 'source'-handler accessing the repo (network, out of memory, ...), or even worse a configuration change you are not aware of. The last one lead to this issue. A windows update to IIS changed a nondescriptive configuration value from true to false. Status 200, SNAFU. mantisbt ms si The most valuable part of source integration ('Fixes #') silently broken.

(I only use the plugin with Github, which does not use this functionality).

So what's the workflow between the mantis github repo and the mantis tracker (source integration) when a commit happens. There's surely an app/hook configured that's triggered on commits. How are this kind of errors tracked there?

dregad commented 6 years ago

I see what you mean now.

OK so the 400 errors would probably only apply to the cases already handled within checkin.php itself (where the die statements are).

500 is correct if it's a server error, but I'm not sure how to catch that within checkin.php; what you report is a standard MantisBT error page, we would probably need to have Mantis core throw an exception instead, to handle this properly.

dregad commented 1 month ago

Problem will probably be fixed by https://mantisbt.org/bugs/view.php?id=34634

obmsch commented 1 month ago

@dregad thanks that works. I now get a 500 in my use case.