scotty-web/scotty

'status' and 'raiseStatus' (and 'redirect') overlap in functionality

ocramz opened this issue · 5 comments

ocramz commented

One could raiseStatus 200 "potato" and it would be thrown as an exception, for example.

Also the redirect mechanism could be improved while we're at this. Eg. if you raiseStatus 308 "/foo/bar" you get redirected.

While a breaking change with the current implementation, the new behaviour would align better with users' expectations.

We can use the predicates defined here to inspect HTTP status codes and act accordingly (i.e. throw an exception or not) : https://hackage.haskell.org/package/http-types-0.12.3/docs/Network-HTTP-Types.html#v:statusIsInformational

if you raiseStatus 308 "/foo/bar" you get redirected.

No please don't do this, because "/foo/bar" is not a response body but a location header (I assume that's your intent). It would be very strange to change the semantics of the second argument depending on the value of the status code.

I think the right direction is to get rid of raiseStatus altogether, and encourage users to use either finish or a user-defined exception

ocramz commented

one problem with deprecating raiseStatus is that we use it internally when a request body is too large.

IMO calling raiseStatus from scotty should be avoided because it makes it harder to customise the response. How about making ScottyException more semantic, providing a default handler in place of statusErrorHandler? For example it could be data ScottyException = RequestTooLarge | MalformedJSON |...

This way, by setting defaultHandlers users are able to customise the handling of these exceptions

ocramz commented

Sure. I'll submit a PR in the coming days