Skip to content

Log unhandled exceptions#13

Open
Raveline wants to merge 3 commits intohaskell-effectful:masterfrom
Raveline:log-unhandled-exceptions
Open

Log unhandled exceptions#13
Raveline wants to merge 3 commits intohaskell-effectful:masterfrom
Raveline:log-unhandled-exceptions

Conversation

@Raveline
Copy link
Copy Markdown

@Raveline Raveline commented Jun 3, 2025

Add an ultimate log if an uncaught exception interrupt the effects being run.

Comment thread src/Effectful/Log.hs Outdated
(`local` unlift action) $ \logEnv -> logEnv { leDomain = leDomain logEnv ++ [domain] }
LocalMaxLogLevel level action -> localSeqUnlift env $ \unlift -> do
(`local` unlift action) $ \logEnv -> logEnv { leMaxLogLevel = level }
GetLoggerEnv -> ask) . handle logException
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use withException instead of handle (needs effectful-core-2.6.0.0).

Comment thread src/Effectful/Log.hs Outdated
, module Log
) where

import Control.Exception.Lifted
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Effectful.Exception.

Comment thread src/Effectful/Log.hs Outdated
time <- liftIO getCurrentTime
logEnv <- getLoggerEnv
liftIO $
logMessageIO logEnv time LogAttention "Uncaught exception" $ object ["error" .= (pack . show $ e)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use logAttention here (and you don't need pack, same as in the log-base PR).

Comment thread src/Effectful/Log.hs Outdated
(`local` unlift action) $ \logEnv -> logEnv { leMaxLogLevel = level }
GetLoggerEnv -> ask
runLog component logger maxLogLevel =
reinterpret reader (\env -> \case
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think putting the handler in the where clause will be better for readability - right now it's somewhat hard to see the "handle" stuff at the end.

Comment thread src/Effectful/Log.hs Outdated
logEnv <- getLoggerEnv
liftIO $
logMessageIO logEnv time LogAttention "Uncaught exception" $ object ["error" .= (pack . show $ e)]
throw e
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throwIO

@arybczak
Copy link
Copy Markdown
Member

Ah, also needs a bump to 1.2.0.0 and a changelog entry.

Copy link
Copy Markdown
Contributor

@mmhat mmhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think I'd prefer if we could leave the original runLog as it is now and introduce a new runLogWithException (or something like that) that logs uncaught exceptions:

runLogWithException component logger maxLevel = runLog component logger maxLevel . handle logException

@arybczak
Copy link
Copy Markdown
Member

Why? 🤔

@mmhat
Copy link
Copy Markdown
Contributor

mmhat commented Sep 11, 2025

Why? 🤔

It's not a strong argument, but I expect the runEffectName to be rather minimal: Handle the effect and do nothing more.
But yes, I'd probably want to go most of my errors to be logged too, so in this case it's rather sticking to a principle than an objection to the concrete change...

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

Successfully merging this pull request may close these issues.

3 participants