silkapp / rest

Packages for defining APIs, running them, generating client code and documentation.
http://silkapp.github.io/rest
390 stars 52 forks source link

Inconsistencies in error handling between backends #61

Closed bergmark closed 9 years ago

bergmark commented 10 years ago

If you have a handler whose body contains say error "foo" all backends handle this differently and only rest-wai does the appropriate thing.

All backends should print the error, it may be less important that the response is the same, but showing the actual error message like rest-snap does is bad.

hesselink commented 10 years ago

Isn't this just how the backends handle exceptions by default?

bergmark commented 10 years ago

Probably, but I think we should change it. At the moment a lot of errors are lost when using rest-happstack.

dpwiz commented 10 years ago

Take a note that error messages can leak sensitive information...

hesselink commented 10 years ago

So this is an issue mostly with happstack, because it uses lazy bytestrings. That means that if there is an undefined or something in the computation that produces a response, you might lose the error, even if you have a top level exception handler around your code. rest inherits this limitation, since it also uses a lazy bytestring. So I think it's up to users to handle this, with a top level exception handler and optionally forcing the response body if you want to be sure you catch everything.

bergmark commented 9 years ago

Is there anything we can actually do about this in rest? It sounds to me that users just have to pick a backend that does this properly. We can probably close this.

hesselink commented 9 years ago

We could switch rest to use some kind of streaming abstraction (pipes, conduit, io-streams). Then I think wai and snap could handle this correctly. I'm not sure how much work this would be. A lot of it would just be choosing what to use and seeing how it impacts the whole framework, I guess.

bergmark commented 9 years ago

wai already does this well enough, and snap-1.0 should also fix it.

hesselink commented 9 years ago

OK, let's close it then.