Lens-based interfaces

[pbrisbin]

Idea 1: Our records should expose lenses.

Instead of:

setMessage :: Text -> BeforeNotify
setMessage msg = updateException $ \ex -> ex
  { beMessage = Just "Something"
  }

You could do

setMessage :: Text -> BeforeNotify
setMessage msg = exception . message ?~ "Something"

Not only does this remove a lot of record-update noise, but it also makes most of our updateException helpers unnecessary.

Idea 2: Document a prism-based approach for the exception hiearchy

:warning: I actually don't know Lens very well. I know enough to know this is possible, but I won't be getting the syntax right.

Instead of:

someBeforeNotify :: BeforeNotify
someBeforeNotify = updateException forSqlError

forSqlError :: BugsnagException -> BugsnagException
forSqlError ex =
    case fromException =<< beOriginalException ex of
        Just SqlError{..} -> ex
            { beErrorClass = "SqlError-" <> sqlErrorCode
            , beMessage = Just sqlErrorMessage
            }
        _ -> ex

You could do (something like):

someBeforeNotify :: BeforeNotify
someBeforeNotify = over (exception . originalException . _SqlError)
    $ (errorClass .~ "SqlError-" <> sqlErrorCode)
    . (message ?~ sqlErrroMessage)

Idea 3: RIO-style Reader+Has+local

We've done this in our work app and it's working pretty well.

class HasBugsnagSettings env where
  bugsnagSettingsL = Lens' env BugsnagSettings

instance HasBugsnagSettings App where
  -- ...

main :: IO ()
main = do
  app <- loadApp -- typical startup, including building BugsnagSettings

  runRIO app $ do
    -- Default behavior
    something `catch` notifyBugsnag

    -- Modify any aspect of Settings for just a section using local 
    local (bugsnagSettingsL . sessionContext %~ "Critical") $ do
      somethingCritical `catch` notifyBugsnag

This obviates notifyBugsnagWith, by the way.

local (bugsnagSettingsL . beforeNotify <>~ something) $ do

Since BeforeNotify is just a function, using (<>~) allows us to compose a new function after the global one. We may need to make BeforeNotify an alias for Endo BugsnagEvent though.

[parsonsmatt]

I think I like the class to be on m rather than env - eg,

class Monad m => MonadBugsnag m where
    askBugsnagSettings :: m BugsnagSettings
    localBugsnagSettings :: (BugsnagSettings -> BugsnagSettings) -> m a -> m a

With a newtype for deriving via, like,

class HasBugsnagSettings a where
    -- get, set, lens, whatever

newtype BugsnagViaReader m a = BugsnagViaReader (m a)
  deriving newtype (Functor, Applicative, Monad)

instance (MonadReader env m, HasBugsnagSettings a) => MonadBugsnag (BugsnagViaReader m) where
  askBugsnagSettings = BugsnagViaReader (asks getBugsnagSettings)
  localBugsnagSettings f (BugsnagViaReader r) = BugsnagViaReader (local (over _ f) r)

then you can either derive from the MonadReader instance and define HasBugsnagSettings, or you can define directly on the m type.

You can give an instance to StateT BugsnagSettings with the MonadBugsnag m, but you cannot do so for MonadReader.

I'm not sure if this is all that well-founded, though.

---

On the topic of lenses, its' nice to export them, but I always like to see regular getters/setters/modifiers exported too for making things beginner-friendly.

[pbrisbin]

That's an interesting trick, to define MonadBugsnag as just a "concrete" (in a way) reader. I'll have to think about that.

I've always seen it more like,

class MonadBugsnag m where
  notify :: Exception e => e -> m ()

-- for deriving via
newtype ActualBugsnag m a = ActualBugsnag (m a)
  deriving newtype (Functor, {- ... -})

instance (MonadReader env m, HasBugsnagSettings env) => MonadBugsnag (ActualBugsnag m) where
  notify ex = do
    settings <- view bugsnagSettingsL
    Bugsnag.notify settings ex

I kind of feel like if you're gonna have a MonadBugsnag m and a derive-able, you might as well make it totally flexible rather than bake-in being Reader like and so only really having ReaderT and StateT instances possible. I could be misunderstanding though -- I feel like I learned these patterns from you anyway so :shrug:

On the topic of lenses, its' nice to export them, but I always like to see regular getters/setters/modifiers exported too for making things beginner-friendly.

That's fair. The new version (the bugsnag package) uses bugsnag-hs for the actual payload data, which means full records with getter/setter fields are exposed (Data.Bugsnag), so lens would indeed be "on top" and "opt in".

[parsonsmatt]

Oh, yeah. That does have advantages - you can write a dodgy instance for MonadBugsnag IO that reads the settings.yaml file and fires off an exception :joy:

[pbrisbin]

Oh yeah, because MonadIO m => MonadIO (SqlPersistT m) have never bitten us, not once.