Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
obmsch opened this issue Apr 22, 2018 · 3 comments
Open

Comments

@obmsch
Copy link
Contributor

obmsch commented Apr 22, 2018

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
Copy link
Member

dregad commented Apr 23, 2018

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 😉... 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
Copy link
Contributor Author

obmsch commented Apr 24, 2018

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
Copy link
Member

dregad commented Apr 24, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants