search

scotty-web / scotty

Haskell web framework inspired by Ruby's Sinatra, using WAI and Warp (Official Repository)
http://hackage.haskell.org/package/scotty
BSD 3-Clause "New" or "Revised" License
1.71k stars 132 forks source link

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

Closed ocramz closed 7 months ago

ocramz commented 9 months ago

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

fumieval commented 9 months ago

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 9 months ago

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

fumieval commented 9 months ago

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 9 months ago

Hm, I cannot visualize this solution at the moment, but I'll be happy to review a PR if you have something specific in mind! :)

On Mon, 16 Oct 2023 at 04:56, Fumiaki Kinoshita @.***> wrote:

IMO calling raiseStatus from scotty should be avoided because it makes 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 |...

— Reply to this email directly, view it on GitHub https://github.com/scotty-web/scotty/issues/338#issuecomment-1763648529, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNBDKGXQK37QWFSJQ33353X7SO6PAVCNFSM6AAAAAA6A32QEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRTGY2DQNJSHE . You are receiving this because you authored the thread.Message ID: @.***>

fumieval commented 9 months ago

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