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

Guidance on using scoped with multiple Polysemy.Error e effects #2

Open
adlaika opened this issue Oct 4, 2022 · 18 comments
Open

Guidance on using scoped with multiple Polysemy.Error e effects #2

adlaika opened this issue Oct 4, 2022 · 18 comments

Comments

@adlaika
Copy link

adlaika commented Oct 4, 2022

I've used Scoped to write a database transaction effect:

data Transaction m a where
  Begin           :: Transaction m ()
  Commit          :: Transaction m ()
  Rollback        :: Transaction m ()

makeSem ''Transaction

interpretDBTransaction :: Member (Embed IO) r => ConnectionPool -> InterpreterFor Transaction r
interpretDBTransaction cPool = interpret $ \case
  Begin -> embed $ withConnectionPool cPool Psql.begin
  Commit -> embed $ withConnectionPool cPool Psql.commit
  Rollback -> embed $ withConnectionPool cPool Psql.rollback

withTransaction :: Members '[
    Error e
  , Scoped resource Transaction
  ] r => InterpreterFor Transaction r
withTransaction act = scoped $ do
  begin
  result <- act `catch` \e -> rollback >> throw e
  commit
  pure result

type ScopedTransaction resource = Scoped resource Transaction

...which works great! However, I'm not sure how to extend it to use two different Polysemy.Error members:

doAThingThatCanThrowMultipleTypesOfError :: Members '[
    Error DomainError
  , Error ElvesLooseInMyComputerError
  ] r
  => Sem r ()
doAThingThatCanThrowMultipleTypesOfError  = withTransaction $ undefined

blows up with

Could not deduce: Polysemy.Internal.Union.LocateEffect
                          (Error e0) r
                        ~ '()
        arising from a use of ‘withTransaction’

Can you point me in the right direction?
Thanks!

@tek
Copy link
Owner

tek commented Oct 4, 2022

It looks to me like it should work by using a type application to specify which error to use, as in:

withTransaction @DomainError do ...

unless you're actually keeping e polymorphic in doAThing…, in which case this is a general problem with polymorphic effects and might require you to use Tagged "foo" (Error e).
If you supply some more context I may be able to figure it out better.

May I ask – why are you running begin/commit/rollback in the scope function instead of in the withResource argument to interpretScoped/runScoped?

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

There are definitely cases where we want to be able to throw either a DomainError or, for instance, a ThirdPartyClientError inside the same transaction block, but Tagged is a good place for me to start--thanks!

For context: it's a standard http web server, and we want most route handlers to be transactional.

As for the second question...because it's the first thing I got to work 😅. Is there a better way to do it?

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

Here's an example of an actual Servant handler we're using:

createReward :: Members '[
    Effect.CreatorStore
  , CreatorRewardStore
  , Error DomainError
  , ScopedTransaction a
  , Log
  ] r
  => AstroUser -> CreatorRewardCreate -> Sem r RewardId
createReward user rewardCreate = withTransaction $ do
  creator <- maybe throwCreatorNotFound pure =<< Effect.getCreatorById creatorId
  authorizeIfUserIdMatchesOrAdmin user (creator^.Lens.ownerId) "createReward"
  maybe throwImpossible pure =<< createCreatorReward rewardCreate
  where
    creatorId = rewardCreate ^. Lens.creatorId
    throwCreatorNotFound = throw $ ErrorCreatorWithIdNotFound creatorId
    impossibleMsg = "createCreatorReward failed: " <> tshow rewardCreate
    throwImpossible = logError impossibleMsg >> throw (ErrorImpossible impossibleMsg)

As you can see, we've had to add ErrorImpossible to the DomainError type in order to represent a non-domain error.

@tek
Copy link
Owner

tek commented Oct 4, 2022

oh, now I see what you are trying to achieve – I didn't look close enough 🙈

For your purpose, the actual exception is irrelevant, since you only want to react to something aborting the computation.
This is a job for the Resource effect – if you just use

onException act rollback

then any error will be caught.

@tek
Copy link
Owner

tek commented Oct 4, 2022

As for the second question...because it's the first thing I got to work sweat_smile. Is there a better way to do it?

After thinking about this some more I realized that your database effects probably use some thread-local state to associate queries with the connection pool on which the transaction is started, is that right?

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

Nothing so complicated, as far as I know (unless beam is doing something magical under the hood). We only use one connection pool. Here's the relevant interpreter:

data DBBeam m a where
  RunPG :: Pg a -> DBBeam m a

makeSem ''DBBeam

dbBeamToIO :: Member (Embed IO) r => ConnectionPool -> InterpreterFor DBBeam r
dbBeamToIO cPool = interpret $ \case
  RunPG p -> embed $ withConnectionPool cPool (`runBeamPostgres` p)

And its place in the interpreter stack:

sem
...
    & runScopedAs (pure pool) interpretDBTransaction
    & dbBeamToIO pool
...

@tek
Copy link
Owner

tek commented Oct 4, 2022

I see…then how do you ensure that multiple threads don't get their transactions tangled up?

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

The sinking feeling in my gut implies that maybe we don't 😅. I guess it's a good thing I reached out--I didn't even consider that issue.

@tek
Copy link
Owner

tek commented Oct 4, 2022

right 🙂
the general workflow for transactions with Scoped would be something like:

acquireConnection ::
  Member ConnectionPool r =>
  (Connection -> Sem r a) ->
  Sem r a
acquireConnection use =
  conn <- ConnectionPool.exclusiveConnection
  bracketOnError Psql.begin (const Psql.rollback) \ () -> do
    a <- use conn
    Psql.commit
    pure a

runScopedDB ::
  Member ConnectionPool r =>
  InterpreterFor (Scoped Connection DBBeam) r
runScopedDB =
  interpretScoped acquireConnection \ conn -> \case
    DBBeam pg -> runBeamPostgres conn pg

but this isn't enough for your case, since (a) you're not using DBBeam directly, but *Store, which I assume produces a Pg in an interpreter, and (b) you're even using multiple *Store effects in a single transaction!
You can chain scopes without much effort, but using multiple arbitrary effects in a single scope isn't easily possible.
I think you'd have to write interpreters for every combination of stores in that case, using something like Polysemy.Bundle:

withDB ::
  Member (Scoped res DBBeam) r =>
  (() -> Sem r a) ->
  Sem r a
withDB use =
  scoped (use ())

runStores :: InterpreterFor (Scoped () (Bundle [Store1, Store2])) r
runStores =
  interpretScoped withDB \ () -> \case
    Bundle Here (Store.Query q) ->
      dBBeam (runQuery q)
    Bundle (There Here) (Store.Query q) ->
      dBBeam (runQuery q)

It's not very comfortable, but maybe you can figure something out 🙂

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

Oh dear. Perhaps it would be better to ditch the *Stores approach then, and go with something more general. Thank you so much, you've given me a lot to think about.

@tek
Copy link
Owner

tek commented Oct 4, 2022

I do use the Store approach myself, but I haven't attempted transactions yet, though I would be really interested to see a solution!

The Bundle method does create some boilerplate overhead, but you might be able to create generic interpreters that delegate to single-store interpreters and only have to be instantiated with a type argument in the final stack. I think it would be worth a try!

@adlaika
Copy link
Author

adlaika commented Oct 4, 2022

I'll give it a shot!

@adlaika
Copy link
Author

adlaika commented Oct 13, 2022

I'm afraid I have to admit I don't think I have the requisite noggin power to implement the Bundle solution, but I do have another question:

how do I interpret a Scoped effect to Polysemy.State in such a way that I can test against the resultant state? For instance...

Test.Hspec.spec :: Spec
Test.Hspec.spec = do
  describe "state test" $ do 
    it "does something stateful" $ do
        actual <- runM 
          $ (doSomethingWithAnEffectIWantToMockAsState :: Sem r String)
          & runScopedAs (pure ()) (\() -> runState [] . (storeToState  :: Sem (Store ': r) String -> Sem (State [Int] ': r) String))
        let expectedState = []
        actual `shouldBe` (expectedState, "the actual result")

...doesn't typecheck, because runScopedAs returns a Sem ((Scoped resource effect) ': r) a -> Sem r a, which clearly can't change the return type to the needed (s, a). Am I missing something, or is there no way to "leak" the resultant state of runState [] out into the larger test scope, short of rewriting runScopedAs or one of the other Scoped interpreters?

@tek
Copy link
Owner

tek commented Oct 14, 2022

It is indeed not possible to do that with the existing interpreters.
It might be feasible to add them, but I would suggest using an MVar instead (like the Sync effect).

An important issue you should be aware of is that this won't work at all with runScopedAs (consult the documentation for details).
Instead, you can use interpretScopedWith:

handleStoreToState :: Member (State [Int]) r => Store m a -> Sem r a
handleStoreToState =
  undefined

scope ::
  Member (Sync [a]) r =>
  (() -> Sem (State [a] : r) x) ->
  Sem r x
scope use = do
  (s, a) <- runState [] (use ())
  a <$ Sync.putTry s

foo :: Sem r ()
foo = do
  r :: Maybe [Int] <- interpretSync $ interpretScopedWith @'[State [Int]] scope (const handleStoreToState) do
    -- test body
    Sync.try
  embed (print r)

As to the Bundle concept, I created a sketch of the pattern for you: https://gist.github.com/tek/155728a934e574b50e41462016cac0b9

@adlaika
Copy link
Author

adlaika commented Oct 17, 2022

Ah, of course--that makes sense. Thank you so much, I really appreciate the time.

@tek
Copy link
Owner

tek commented Oct 17, 2022

my pleasure! btw, I just released a new version of Scoped without the resource param (which will also soon be released in the mainline polysemy package).

@qwbarch
Copy link

qwbarch commented Jan 28, 2023

@adlaika Did you end up sticking to the stores approach? I'm looking to do the exact same and then stumbled into this issue

@tek
Copy link
Owner

tek commented Jan 28, 2023

FYI I implemented a variant that's a bit more ergonomic while redesigning my hasql library, though it's still under construction but works fine:

should be possible to be generalized/used with other backends.

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

No branches or pull requests

3 participants