source-integration icon indicating copy to clipboard operation
source-integration copied to clipboard

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

Open obmsch opened this issue 6 years ago • 3 comments

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.

obmsch avatar Apr 22 '18 09:04 obmsch

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).

dregad avatar Apr 23 '18 13:04 dregad

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?

obmsch avatar Apr 24 '18 10:04 obmsch

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 avatar Apr 24 '18 11:04 dregad