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

AddSetCookies missing an instance for when the left tree is the same before and after the transformation #1601

Closed
mastarija opened this issue Aug 13, 2022 · 9 comments

Comments

@mastarija
Copy link

So, I've got this error

server/Server/SRV.hs:63:14: error:
    • Overlapping instances for Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                  ('Servant.Auth.Server.Internal.AddSetCookie.S
                                     ('Servant.Auth.Server.Internal.AddSetCookie.S
                                        'Servant.Auth.Server.Internal.AddSetCookie.Z))
                                  ((Data.Tagged.Tagged Handler Network.Wai.Application
                                    Servant.API.Alternative.:<|> ((Data.Text.Internal.Text
                                                                   -> Data.Tagged.Tagged
                                                                        Handler
                                                                        Network.Wai.Application)
                                                                  Servant.API.Alternative.:<|> Data.Tagged.Tagged
                                                                                                 Handler
                                                                                                 Network.Wai.Application))
                                   Servant.API.Alternative.:<|> (Data.Tagged.Tagged
                                                                   Handler Network.Wai.Application
                                                                 Servant.API.Alternative.:<|> ((Data.Text.Internal.Text
                                                                                                -> Data.Tagged.Tagged
                                                                                                     Handler
                                                                                                     Network.Wai.Application)
                                                                                               Servant.API.Alternative.:<|> Server.API.PdxfAPI.PdxfAPI
                                                                                                                              Flouble
                                                                                                                              (AsServerT
                                                                                                                                 Handler))))
                                  ((Data.Tagged.Tagged Handler Network.Wai.Application
                                    Servant.API.Alternative.:<|> ((Data.Text.Internal.Text
                                                                   -> Data.Tagged.Tagged
                                                                        Handler
                                                                        Network.Wai.Application)
                                                                  Servant.API.Alternative.:<|> Data.Tagged.Tagged
                                                                                                 Handler
                                                                                                 Network.Wai.Application))
                                   Servant.API.Alternative.:<|> (Data.Tagged.Tagged
                                                                   Handler Network.Wai.Application
                                                                 Servant.API.Alternative.:<|> ((Data.Text.Internal.Text
                                                                                                -> Data.Tagged.Tagged
                                                                                                     Handler
                                                                                                     Network.Wai.Application)
                                                                                               Servant.API.Alternative.:<|> Data.Tagged.Tagged
                                                                                                                              Handler
                                                                                                                              Network.Wai.Application)))
        arising from a use of ‘genericServeTWithContext’
      Matching instances:
        two instances involving out-of-scope types
          instance [overlappable] (Functor m,
                                   Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                     n (m old) (m cookied),
                                   Servant.API.ResponseHeaders.AddHeader
                                     "Set-Cookie" Web.Cookie.SetCookie cookied new) =>
                                  Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                    ('Servant.Auth.Server.Internal.AddSetCookie.S n) (m old) (m new)
            -- Defined in ‘Servant.Auth.Server.Internal.AddSetCookie’
          instance [overlap ok] (Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                   ('Servant.Auth.Server.Internal.AddSetCookie.S n) a a',
                                 Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                   ('Servant.Auth.Server.Internal.AddSetCookie.S n) b b') =>
                                Servant.Auth.Server.Internal.AddSetCookie.AddSetCookies
                                  ('Servant.Auth.Server.Internal.AddSetCookie.S n)
                                  (a Servant.API.Alternative.:<|> b)
                                  (a' Servant.API.Alternative.:<|> b')
            -- Defined in ‘Servant.Auth.Server.Internal.AddSetCookie’
    • In the second argument of ‘($)’, namely
        ‘genericServeTWithContext tran srv ctx’
      In a stmt of a 'do' block:
        run 8088 $ genericServeTWithContext tran srv ctx
      In the second argument of ‘($)’, namely
        ‘do let jwk = hs256jwk "aXTrwbg2peHxiY6JKAXVX8kFrcPZ2Mto"
                ctx
                  = defaultCookieSettings :. defaultJWTSettings jwk :. EmptyContext
            run 8088 $ genericServeTWithContext tran srv ctx’
   |
63 |   run 8088 $ genericServeTWithContext tran srv ctx
   |

If we look carefully, it's saying that for this type:

AddSetCookies
  ('S ('S 'Z))
  (
    (
      Tagged Handler Application
      :<|>
      (
        (Text -> Tagged Handler Application)
        :<|>
        Tagged Handler Application
      )
    )
    :<|>
    (
      Tagged Handler Application
      :<|>
      (
        ( Text -> Tagged Handler Application)
        :<|>
        PdxfAPI Flouble (AsServerT Handler)
      )
    )
  )

  (
    (
      Tagged Handler Application
      :<|>
      (
        (Text -> Tagged Handler Application)
        :<|>
        Tagged Handler Application
      )
    )
    :<|>
    (
      Tagged Handler Application
      :<|>
      (
        ( Text -> Tagged Handler Application)
        :<|>
        Tagged Handler Application
      )
    )
  )

It cannot decide if it should use AddSetCookies ('S n) (m old) (m new) or AddSetCookies ('S n) (a :<|> b) (a' :<|> b').

This is because the first branch is the same e.g. AddSetCookies ('S n) (a:<|>b) (a :<|>b) and therefore a :<|> can be interpreted as m in AddSetCookies ('S n) (m old) (m new).

It's seems perfectly reasonable to me that the a branch can stay the same after the AddSetCookie transformation. I've written this orphan instance that fixed my issue.

instance {-# OVERLAPPING #-}
  ( AddSetCookies ('S n) a a
  , AddSetCookies ('S n) b b'
  )
  => AddSetCookies ('S n) (a :<|> b) (a :<|> b') where
  addSetCookies cookies ( a :<|> b) = addSetCookies cookies a :<|> addSetCookies cookies b
@mastarija
Copy link
Author

If someone more experienced with servant could confirm that this is indeed the case and this instance should exist (and it's not just working by accident) then I'd be happy to do a pull request for this issue.

@silky
Copy link

silky commented Nov 28, 2022

is this not solved by this - #1531 ?

@mastarija
Copy link
Author

@silky I'm not sure. #1531 was merged in February, but I still experienced this problem in August. :/

@silky
Copy link

silky commented Nov 30, 2022

i see

well, i'm no expert, but your solution certainly seems reasonable to me :)

@endgame
Copy link
Contributor

endgame commented Jul 28, 2023

A colleague and I hit this problem with one of our API types and spent a good amount of time looking at the proposed new instance, as well as reading the GHC description of overlapping instance pragmas. I'm pretty sure this is correct, and #1653 is probably not what's wanted.

@mastarija
Copy link
Author

@endgame well, I guess that's enough encouragement for me to open a PR then :). I'll try to do it over the weekend.

@tchoutri
Copy link
Contributor

Fantastic, thank you both

@endgame
Copy link
Contributor

endgame commented Feb 6, 2024

#1702 was merged. Should this be closed?

@mastarija
Copy link
Author

@endgame I think so

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

4 participants