uSked / mosaico-php-backend

A PHP backend for Mosaico
GNU General Public License v3.0
32 stars 34 forks source link

Implement a proper error handling approach #4

Open bwl21 opened 8 years ago

bwl21 commented 8 years ago

see https://github.com/voidlabs/mosaico/issues/45

I am not aware if mosaico is prepared for handling of server errors. But mosaico-php-backend should capture php errors and provide a proper response.

mherbold commented 8 years ago

mosaico-php-backend is a direct 1 to 1 translation from the default mosaico node.js backend code. If there isn't any error response in the php backend it is because there isn't one in the node.js backend either.

bwl21 commented 8 years ago

I have opened https://github.com/voidlabs/mosaico/issues/45 to discuss and agree upon an approach. I plan to drive your PHP backend to production which in my opinion requires a stable error handling.

mherbold commented 8 years ago

It is not up to me - I don't run the Mosaico project. The people running the Mosaico project would need to add error handling to their project and update their example back end to show what the proper JSON response would be for errors.

Is there a specific error you are getting that you are trying to fix?

mherbold commented 8 years ago

Sorry I just re-read your last comment and see that you have already started a thread on voidlabs/mosaico to bring this issue up with them. I do agree that there is a real lack of error handling that should be addressed.

mherbold commented 8 years ago

I now capture more errors and return different http response codes. Things should be better now.

bwl21 commented 8 years ago

Had a look in the code (did not try yet). The refactored architecture still does respond with 200 if Image resizing failed.

mherbold commented 8 years ago

Can you tell me what line the resize fails on for you? Or share the image that it is failing to resize the image with. If I can reproduce the fail then I can figure out where to detect the error and then set $http_return_code = 500 and return.

bwl21 commented 8 years ago

If everything is configured correctly, image resizing works fine. But if not it should give a 500. There are some error cases such as missing access rights, exceeded quota etc. Maybe it is worth to wrap a try/catch around it.