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

Add custom type errors #576

Open
fizruk opened this Issue Aug 22, 2016 · 13 comments

Comments

Projects
None yet
7 participants
@fizruk
Member

fizruk commented Aug 22, 2016

We all know servant's type errors are awfully long and hard to read (especially for newbies).
Let's change that with new feature of GHC 8 — custom type errors!

I don't have anything concrete yet, rather just clearing my head of this.
I don't think this is a very hard task, but probably not newcomer-friendly.

@jkarni

This comment has been minimized.

Member

jkarni commented Aug 22, 2016

It would be great. The only thing we have to figure out is how to test this: the only way simple I can think of doing it is with doctest, but we'd need to branch on GHC version. I guess we can CPP documentation?

@fizruk

This comment has been minimized.

Member

fizruk commented Aug 22, 2016

IIRC when you CPP documentation, you have to CPP the whole block.
You can see this in http-api-data's sources.

However, we could add a separate named haddock chunk with doctests and description of nice errors.

@soenkehahn

This comment has been minimized.

Contributor

soenkehahn commented Aug 25, 2016

Since doctest is meant to be read as a ghci session I think it's generally acceptable to stick to one version of ghc. Otherwise we would have to provide different documentation for different ghc versions, which sounds unpractical. So I would consider just disabling doctest for all but one ghc version.

In this special case, we should prominently point out that the type error messages reflect what ghc-8 outputs. And that previous versions might yield much different (and much worse) type errors.

@jkarni

This comment has been minimized.

Member

jkarni commented Aug 25, 2016

+1. I think a positive feedback loop with regards to better error messages is
us showing how much better things are in GHC 8, so that more of our users
switch to GHC 8, so that the impact of writing better custom errors becomes
larger, &c.

On Thu, Aug 25, 2016 at 06:31:02AM -0700, Sönke Hahn wrote:

Since doctest is meant to be read as a ghci session I think it's generally acceptable to stick to one version of ghc. Otherwise we would have to provide different documentation for different ghc versions, which sounds unpractical. So I would consider just disabling doctest for all but one ghc version.

In this special case, we should prominently point out that the type error messages reflect what ghc-8 outputs. And that previous versions might yield much different (and much worse) type errors.

You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#576 (comment)

Julian K. Arni
Haskell Consultant, Turing Jump
https://turingjump.com

@arianvp

This comment has been minimized.

Member

arianvp commented Sep 2, 2016

Please paste any horrible error messages here! :)

@arianvp

This comment has been minimized.

@chkl

This comment has been minimized.

Contributor

chkl commented Sep 2, 2016

/home/klingerc/servant/example-servant-custom-monad/src/App.hs:47:10:
    Couldn't match type ReaderT
                           AppConfiguration Data.Functor.Identity.Identity [Issue]
                         :<|> (IssueId
                               -> ReaderT AppConfiguration Data.Functor.Identity.Identity Issue)
                   with ReaderT
                           AppConfiguration Data.Functor.Identity.Identity [Issue]
    Expected type: ServerT API MyMonad
      Actual type: ReaderT
                     AppConfiguration Data.Functor.Identity.Identity [Issue]
    In the expression: hListIssues <|> hGetIssue
    In an equation for server’:
        server
          = hListIssues <|> hGetIssue
          where
              hListIssues :: MyMonad [Issue]
              hListIssues = undefined
              hGetIssue :: IssueId -> MyMonad Issue
              hGetIssue = undefined

/home/klingerc/servant/example-servant-custom-monad/src/App.hs:47:26:
    Couldn't match expected type ReaderT
                                    AppConfiguration Data.Functor.Identity.Identity [Issue]
                with actual type IssueId -> MyMonad Issue
    Probable cause: hGetIssue is applied to too few arguments
    In the second argument of (<|>), namely hGetIssue
    In the expression: hListIssues <|> hGetIssue

after confusing <|> with :<|>

@arianvp

This comment has been minimized.

Member

arianvp commented Sep 2, 2016

@chkl hmm I think this one is hard to improve as the mistake is made on the term level and not in the API definition

@arianvp

This comment has been minimized.

@rikvdkleij

This comment has been minimized.

rikvdkleij commented Sep 4, 2016

Preprocessing test suite 'spec' for servant-server-0.8.1...
[ 8 of 10] Compiling Servant.ServerSpec ( test/Servant/ServerSpec.hs, .stack-work/dist/x86_64-linux/Cabal-1.24.0.0/build/spec/spec-tmp/Servant/ServerSpec.o )

/home/rik/haskell/libraries/servant/servant-server/test/Servant/ServerSpec.hs:72:5: error:
    • Overlapping instances for Servant.Server.Internal.Context.HasContextEntry
                                  '[AuthHandler Request String, BasicAuthCheck String,
                                    NamedContext "foo" '[]]
                                  (AuthHandler Request (AuthServerData (AuthProtect "foo")))
        arising from a use of ‘serveWithContext’
      Matching instances:
        two instances involving out-of-scope types
        (use -fprint-potential-instances to see them all)
      (The choice depends on the instantiation of ‘’
       To pick the first instance above, use IncoherentInstances
       when compiling the other instance declarations)
    • In the expression:
        serveWithContext comprehensiveAPI comprehensiveApiContext
      In a pattern binding:
        _ = serveWithContext comprehensiveAPI comprehensiveApiContext

Because we forgot the following line:

type instance AuthServerData (AuthProtect "foo") = String
@jkarni

This comment has been minimized.

Member

jkarni commented Sep 4, 2016

Ouch! Fixing @rikvdkleij 's error would be nice.

@fizruk

This comment has been minimized.

Member

fizruk commented Sep 4, 2016

@jkarni we've tried a few things.

First we've tried adding a type instance AuthServerData a = TypeError ..., but that then it conflicts with actual instances. We then tried to wrap AuthServerData type family in a class to be able to use an OVERLAPPABLE instance for any type a, but that didn't help (the conflict remained).

I think there is a hacky way to overcome this... but I have to try it out first.

rikvdkleij added a commit to rikvdkleij/servant that referenced this issue Sep 5, 2016

corpix added a commit to cryptounicorns/mammoth that referenced this issue Feb 20, 2018

Merging broken code just for history
I am giving up on servant because errors are too cryptic, I have already spent a huge amount of hours staring at em and messing around to fix but this delivers nothing but annoyance. At this moment haskell-servant/servant#576 exists and servant errors are subject to change. But this is in the future.
Rewriting service in golang because it is stupidly simple.
Will play with haskell in some other project when infrastructure become better.
@domenkozar

This comment has been minimized.

Contributor

domenkozar commented Jul 9, 2018

I've found generics help quite a bit with that, depending on where things go wrong. Servant 0.14.1 shipped https://haskell-servant.readthedocs.io/en/stable/cookbook/generic/Generic.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment