[CORE-7914] Log an unhandled exception or a user abort before the computation ends by Raveline · Pull Request #97 · scrive/log

Raveline · 2025-06-02T15:11:43Z

Ensures that uncaught exception leaves a log, so it's easier to know what happened and debuggers are not left in the dark. Unfortunately, this adds a constraint to the underlying monad.

…putation ends

arybczak · 2025-06-03T01:55:29Z

log-base/src/Log/Monad.hs

    ask   = lift ask
    local = mapLogT . local

-- | Run a 'LogT' computation.


Why did you remove the haddock?

Because I'm stupid 😬

arybczak · 2025-06-03T01:57:52Z

log-base/src/Log/Monad.hs

 import Control.Monad.Writer.Class
 import Data.Aeson
-import Data.Text (Text)
+import Data.Text (Text, pack)


You don't need pack, aeson handles string conversions itself.

arybczak · 2025-06-03T02:00:17Z

log-base/src/Log/Monad.hs

+          logAttention "Uncaught exception raised" $ object ["error" .= (pack . show $ e)]
+          throwM e
+        ExitCaseAbort ->
+          logAttention_ "Process was aborted manually"


ExitCaseAbort does not mean process was aborted, it means the computation was aborted with means other than runtime exception (like the error from ExceptT if the stack happens to have it).

arybczak · 2025-06-03T02:24:00Z

Please check that it logs the exception when cron exits with the exception (and provide associated kontrakcja PR).

Raveline · 2025-06-03T16:06:35Z

You were right to ask, because it doesn't work :(
Presumably my generalBracket implementation is faulty ? What about relyong on MonadCatch (one rank below Mask), catching any exception, logging it, and rethrowing it ?

Raveline · 2025-06-04T14:17:24Z

All right, finally manage to make it work.

We probably must create a dedicated variant of runLogT that handles exception, lest some of our dependency break due to the added constraints. So you must explicit require a runner that logs exceptions;
I'm afraid we'll also have to create variants in scrive-commons, unless we can find a way around our use of Servant's DelayedIO, which doesn't offer any useful instance for us: https://hackage.haskell.org/package/servant-server-0.17/candidate/docs/Servant-Server-Internal-DelayedIO.html#t:DelayedIO
I also suggest using MonadCatch rather than MonadMask and simply using catch though. The implementation is much simpler and less confusing, plus it is a more "common" monad.

Raveline · 2025-06-11T16:21:39Z

OK, here is a (hopefully) final version using MonadBaseControl. @arybczak, I've tested it with the full pipeline and it works.

theophile-scrive · 2025-06-18T12:35:44Z

I'm afraid we'll also have to create variants in scrive-commons, unless we can find a way around our use of Servant's DelayedIO, which doesn't offer any useful instance for us: hackage.haskell.org/package/servant-server-0.17/candidate/docs/Servant-Server-Internal-DelayedIO.html#t:DelayedIO

Anything that I can help with for that particular issue?

arybczak

Looks good, thanks 👍

Also needs a version bump to 0.13.0.0 and a changelog entry.

arybczak · 2025-06-23T08:54:55Z

log-base/src/Log/Monad.hs

+runLogT component logger maxLogLevel m = runReaderT
+  (unLogT $ liftedCatch m (\(SomeException e) -> do
+      logAttention "Uncaught exception" $ object ["error" .= show e]
+      E.throw e)


Suggested change

E.throw e)

liftBase $ E.throwIO e)

arybczak · 2025-06-23T08:58:46Z

Anything that I can help with for that particular issue?

@theophile-scrive You can add MonadCatch and MonadMask instances for DelayedIO when you have a moment.

arybczak · 2025-06-23T19:16:40Z

Please don't merge yet, I'm wondering about this comment.

Rather than patching runLogT directly, add a composable utility

arybczak

Please fix missing dots at the end of sentences.

arybczak · 2025-06-24T12:19:10Z

log-base/log-base.cabal

 cabal-version:       3.0
 name:                log-base
-version:             0.12.0.1
+version:             0.13.0.0


It can be 0.12.1.0 now since we're not changing runLogT.

arybczak · 2025-06-24T12:20:07Z

log-base/CHANGELOG.md

@@ -1,3 +1,6 @@
+# log-base-0.13.0.0 (2025-06-23)


Suggested change

# log-base-0.13.0.0 (2025-06-23)

# log-base-0.12.1.0 (2025-??-??)

I'll set the date when a release is made.

arybczak · 2025-06-24T12:21:00Z

log-base/src/Log/Monad.hs

  , runLogT
  , mapLogT
  , logMessageIO
+  , logExceptions


Put it after runLogT on the list so it matches place in the code.

arybczak · 2025-06-24T12:23:10Z

log-base/src/Log/Monad.hs

 } -- We can't do synchronisation here, since 'runLogT' can be invoked
  -- quite often from the application (e.g. on every request).

+-- | Unsure uncaught exceptions get logged


arybczak · 2025-06-24T12:23:29Z

log-elasticsearch/log-elasticsearch.cabal

                       http-client,
                       http-types,
-                       log-base >= 0.10 && <0.13,
+                       log-base >= 0.10 && <0.14,


Don't need this now (same below).

Raveline · 2025-06-24T12:39:33Z

Applied your suggestions and added the missing dots.

arybczak

Thank you 👍

Raveline requested a review from arybczak June 2, 2025 15:11

[CORE-7914] Log an unhandled exception or a user abort before the com…

366add1

…putation ends

Raveline force-pushed the log-unhandled-exception-and-user-abort branch from 7324698 to 366add1 Compare June 2, 2025 15:14

arybczak reviewed Jun 3, 2025

View reviewed changes

Apply PR suggestions

ab30e99

Raveline added 4 commits June 4, 2025 09:37

[EXPERIMENT] Try with catch

e9c4bb4

Trying something

33b5d3d

Have a dedicated runner that logs on exception

60cd442

Retrying with MonadMask

d7d8cc2

Use MonadBaseControl IO

c75ec4f

Raveline force-pushed the log-unhandled-exception-and-user-abort branch from c84f054 to c75ec4f Compare June 11, 2025 15:33

arybczak approved these changes Jun 23, 2025

View reviewed changes

Raveline added 2 commits June 23, 2025 17:10

Apply PR suggestions

ff00dce

Bump version and update changelog

cf22190

Provide exceptions logging as a function

11a85b4

Rather than patching runLogT directly, add a composable utility

arybczak reviewed Jun 24, 2025

View reviewed changes

Apply PR suggestions

c8f1072

Raveline force-pushed the log-unhandled-exception-and-user-abort branch from 00ae1a2 to c8f1072 Compare June 24, 2025 12:39

Raveline and others added 2 commits June 26, 2025 14:02

Add missing dots

d93c066

Change error to exception

dacd3a5

arybczak approved these changes Jun 26, 2025

View reviewed changes

fix changelog

b0f8384

arybczak merged commit 36318d6 into master Jun 26, 2025
8 checks passed

arybczak deleted the log-unhandled-exception-and-user-abort branch June 26, 2025 12:53

	# log-base-0.13.0.0 (2025-06-23)
	# log-base-0.12.1.0 (2025-??-??)

Comments

Conversation

Raveline commented Jun 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arybczak commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Raveline commented Jun 3, 2025

Uh oh!

Raveline commented Jun 4, 2025

Uh oh!

Raveline commented Jun 11, 2025

Uh oh!

theophile-scrive commented Jun 18, 2025

Uh oh!

arybczak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arybczak commented Jun 23, 2025

Uh oh!

arybczak commented Jun 23, 2025

Uh oh!

arybczak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Raveline commented Jun 24, 2025

Uh oh!

arybczak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arybczak commented Jun 3, 2025 •

edited

Loading