search

pbrisbin / bugsnag-haskell

Bugsnag error reporter for Haskell
10 stars 7 forks source link

Orphan instances #48

Closed MaxGabriel closed 2 years ago

MaxGabriel commented 5 years ago

This isn't causing any particular problems for me, but I noticed the other day that the library declares two orphan ToJSON instances

instance ToJSON ByteString where
    toJSON = String . decodeUtf8

instance ToJSON a => ToJSON (CI a) where
    toJSON = toJSON . CI.original

I'm not overly opposed to orphans, but the ByteString one seems really presumptuous given that a ByteString could be a JPEG or non UTF-8 text or something. It looks like the orphans are used for e.g. the BugsnagRequest, do you think those might be better as IP (from Data.IP) and Text? Or a newtype wrapper around ByteString?

 data BugsnagRequest = BugsnagRequest
    { brClientIp :: Maybe ByteString
    , brHeaders :: Maybe RequestHeaders
    , brHttpMethod :: Maybe Method
    , brUrl :: Maybe ByteString
    , brReferer :: Maybe ByteString
    }
pbrisbin commented 5 years ago

Yup, I don't disagree. This was just a short-cut taken because of the early nature of the library -- with the intention of cleaning up later.

I'm not sure when I'll have the time, so if you want to make a PR please feel free. I'm not super opinionated about how the orphans are removed (conversion to Text, newtypes, whatever), so feel free to make your own choices.

pbrisbin commented 2 years ago

Closed by #73 and released in v0.0.4.4.