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

Refactor ActionT to use ReaderT pattern #314

Merged
merged 18 commits into from
Oct 2, 2023

Conversation

ocramz
Copy link
Collaborator

@ocramz ocramz commented Sep 28, 2023

scotty-0.20 :

  • Rewrite ActionT using the "ReaderT pattern" : https://www.fpcomplete.com/blog/readert-design-pattern/
  • Clarify the exception handling mechanism of ActionT, ScottyT. The StatusError type can be thrown and caught, but Next, Finish and Redirect cannot be caught by the user. For StatusError and any user-defined (or imported) exception types, one can either catch exceptions inline with rescue or globally for the whole app with defaultHandler.

Breaking changes:

  • Introduce unliftio as a dependency, and base exception handling on catch.
  • rescue changes signature to use proper Exception types rather than strings.
  • All ActionT methods (text, html etc.) have now a MonadIO constraint on the base monad rather than Monad because the Response is in a TVar inside ActionEnv. rescue has a MonadUnliftIO constraint. The Alternative instance of ActionT is also based on MonadUnliftIO because <|> is implemented in terms of catch.

Closes #310 , closes #309 in passing

closes #110 , closes #153

image

@ocramz ocramz marked this pull request as ready for review October 2, 2023 06:09
@ocramz
Copy link
Collaborator Author

ocramz commented Oct 2, 2023

it works omg it w o r k s I'm so happeeeehhh

@ocramz ocramz changed the title WIP refactor ActionT to use ReaderT pattern Refactor ActionT to use ReaderT pattern Oct 2, 2023
@fumieval
Copy link
Collaborator

fumieval commented Oct 2, 2023

Noice

Web/Scotty.hs Outdated Show resolved Hide resolved
Web/Scotty/Exceptions.hs Outdated Show resolved Hide resolved
@ocramz ocramz requested a review from fumieval October 2, 2023 15:54
@ocramz ocramz merged commit 2dc19af into scotty-web:master Oct 2, 2023
5 checks passed
@ocramz ocramz deleted the actiont-readert-#310 branch October 2, 2023 21:42
@fumieval
Copy link
Collaborator

fumieval commented Oct 3, 2023

@ocramz Is it intentional that you merged this PR? Before merging this PR, At very least I expected you to

  • Clean up fixme/WIP commits by git rebase I see the commits squashed on master, that's great
  • Wait for other maintainers' approval

@ocramz
Copy link
Collaborator Author

ocramz commented Oct 3, 2023

@fumieval yes sorry for not waiting but I triple checked everything, added more tests around the new exception mechanism and made a Hackage release. These were long-due changes anyway..

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