Do not merge: Remove Integration Monad #6346

Jimbo4350 · 2025-10-21T17:42:45Z

Description

Remove Integration monad from functions that we also want to use outside of property tests.
Introduce annotated exceptions (via liftIOAnnotated) where the Integration monad has been removed in order to preserve debugging information.

┃ hprop_dump_config :: H.Property
    38 ┃ hprop_dump_config = integrationRetryWorkspace 2 "dump-config-files" $ \tmpDir -> H.runWithDefaultWatchdog_ $ do
       ┃ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       ┃ │ ━━━ Exception (AnnotatedException SomeException) ━━━
       ┃ │ ! AnnotatedException !
       ┃ │ Underlying exception type: IOException
       ┃ │ 
       ┃ │ unsupported operation (Operation is not supported)
       ┃ │ 
       ┃ │ CallStack (from HasCallStack):
       ┃ │   liftIOCallStack, called at src/Testnet/Components/Configuration.hs:186:24 in cardano-testnet-10.0.1-inplace:Testnet.Components.Configuration
       ┃ │   createSPOGenesisAndFiles, called at src/Testnet/Start/Cardano.hs:133:8 in cardano-testnet-10.0.1-inplace:Testnet.Start.Cardano
       ┃ │   createTestnetEnv, called at test/cardano-testnet-test/Cardano/Testnet/Test/DumpConfig.hs:48:23 in cardano-testnet-10.0.1-inplace-cardano-testnet-test:Cardano.Testnet.Test.DumpConfig
       ┃ │   testCase, called at src/Hedgehog/Extras/Test/TestWatchdog.hs:166:66 in hedgehog-extras-0.8.0.0-26dc8622927009e7df9ba3764930f3c64dfd8a2e158af1686ffeb231b1a5219b:Hedgehog.Extras.Test.TestWatchdog
       ┃ │   a given constraint, called at src/Hedgehog/Extras/Test/TestWatchdog.hs:159:26 in hedgehog-extras-0.8.0.0-26dc8622927009e7df9ba3764930f3c64dfd8a2e158af1686ffeb231b1a5219b:Hedgehog.Extras.Test.TestWatchdog
       ┃ │   testCase, called at src/Hedgehog/Extras/Test/TestWatchdog.hs:144:11 in hedgehog-extras-0.8.0.0-26dc8622927009e7df9ba3764930f3c64dfd8a2e158af1686ffeb231b1a5219b:Hedgehog.Extras.Test.TestWatchdog
       ┃ │   runWithWatchdog, called at src/Hedgehog/Extras/Test/TestWatchdog.hs:159:26 in hedgehog-extras-0.8.0.0-26dc8622927009e7df9ba3764930f3c64dfd8a2e158af1686ffeb231b1a5219b:Hedgehog.Extras.Test.TestWatchdog
       ┃ │   runWithDefaultWatchdog, called at src/Hedgehog/Extras/Test/TestWatchdog.hs:166:36 in hedgehog-extras-0.8.0.0-26dc8622927009e7df9ba3764930f3c64dfd8a2e158af1686ffeb231b1a5219b:Hedgehog.Extras.Test.TestWatchdog
       ┃ │   runWithDefaultWatchdog_, called at test/cardano-testnet-test/Cardano/Testnet/Test/DumpConfig.hs:38:82 in cardano-testnet-10.0.1-inplace-cardano-testnet-test:Cardano.Testnet.Test.DumpConfig

Checklist

Commit sequence broadly makes sense and commits have useful messages
New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- roundtrip tests
- integration tests
  See Runnings tests for more details
Any changes are noted in the CHANGELOG.md for affected package
The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-9.6 and ghc-9.12
Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

cardano-testnet/src/Testnet/Orphans.hs

cardano-testnet/src/Testnet/Process/RunIO.hs

carbolymer · 2025-11-04T16:35:25Z

cardano-testnet/src/Testnet/Process/RunIO.hs

+
+liftIOAnnotated :: (HasCallStack, MonadIO m) => IO a -> m a 
+liftIOAnnotated action = GHC.withFrozenCallStack $
+  liftIO $ action `catch` (\(e :: SomeException) -> throwM $ exceptionWithCallStack e)


This is catch from Prelude right? So this should work with watchdog exceptions I think. Could you add a comment here that we're catching async exceptions here as well intentionally?

carbolymer · 2025-11-04T17:10:19Z

cardano-testnet/src/Testnet/Process/RunIO.hs

I'm not a fan of copying code downstream. Have you given a thought how we could reconcile your changes with hedgehog-extras, so possibly we could avoid the duplication?

My main issue with that is having to fix issues in both places, because we're using similar code in cardano-cli as well.

I'll open a PR.

carbolymer · 2025-11-04T17:22:34Z

cardano-testnet/src/Testnet/Runtime.hs

    -- The port number if it is obtained using 'H.randomPort', it is firstly bound to and then closed. The closing
    -- and release in the operating system is done asynchronously and can be slow. Here we wait until the port
-    -- is out of CLOSING state.
-    H.note_ $ "Waiting for port " <> show port <> " to be available before starting node"


This was a useful trace. I don't remember if we weren't experiencing hangups long time ago at this place. In general it would be nice to have a tracer available here so we could emit traces about the progress of the node startup.

cardano-testnet/src/Testnet/Runtime.hs

carbolymer · 2025-11-05T11:42:12Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+liftToIntegration :: RIO ResourceMap a -> H.Integration a 
+liftToIntegration  r = do 
+   rMap <- lift $ lift getInternalState 
+   liftIOAnnotated $ runRIO rMap r


Shouldn't that be evalIO instead? Or at least evalIO . liftIOAnnotated?

Suggested change

liftIOAnnotated $ runRIO rMap r

H.evalIO $ runRIO rMap r

I think regular liftIO (which inside liftIOAnnotated) was losing some location information for me when reporting in the hedgehog. I don't know if that's still the case here.

#6346 (comment)

yes, good catch

carbolymer · 2025-11-05T11:49:13Z

cardano-testnet/src/Testnet/Start/Cardano.hs

            -- Here we assume, very optimistically, that the user has already
            -- instantiated it with a concrete topology file.
-            H.note_ $ "Could not decode topology file. This may be okay. Reason for decoding failure is:\n" ++ e
+            -- TODO: It is suspicious that this decoding can fail. Investigate further.


This will be irrelevant very soon, because non-p2p topology is gone and this double decoding will be removed in: https://github.com/IntersectMBO/cardano-node/pull/6331/files#diff-079ade366e18980b6232319946cc005904a8c865dadf467951da1d30f2069055R268

That's why it would be better to rebase this PR onto 10.6 integration PR. I expect you'll have a lot of conflicts.

The comment needs an update though.

I'm waiting for the 10.6 PR to get merged into master so I will only have to rebase once.

cardano-testnet/src/Testnet/Start/Types.hs

carbolymer · 2025-11-05T12:01:21Z

cardano-testnet/src/Testnet/Start/Types.hs

  , mkConf
+  , mkConfigAbs
+  , mkConfig


So we have mkConf, mkConfig and mkConfigAbs now. This is a bit confusing. I'm wondering if the naming could be improved here.

I removed mkConf from the exports and added a haddock.

cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/SanityCheck.hs

carbolymer · 2025-11-05T12:05:27Z

cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/SanityCheck.hs

+        (_,asyncA) <- asyncRegister_ (threadDelay 10_000_000)
+        let tId = asyncThreadId asyncA
+        return (s,tId)
+     afterForkedThread <- getCurrentTime


FYI You can use https://hackage-content.haskell.org/package/hedgehog-extras-0.10.1.0/docs/Hedgehog-Extras-Test-TestWatchdog.html to verify if code paths were reached.

This differs from hedgehog-extras's asyncRegister function in that the thread is linked before it is cancelled.

cardanoTestnet

…teLogging

carbolymer · 2025-11-06T14:35:22Z

cardano-testnet/src/Testnet/Process/RunIO.hs

+        Nothing -> error $ "missing \"bin-file\" key in plan component: " <> show component <> " in the plan in: " <> planJsonFile
+      [] -> error $ "Cannot find \"component-name\" key with the value \"exe:" <> pkg <> "\" in the plan in: " <> planJsonFile
+    Left message -> error $ "Cannot decode plan in " <> planJsonFile <> ": " <> message
+  where matching :: Component -> Bool


This is an old buggy version of binDist which sometimes is unable to find an executable in plan.json. A fixed one is here: https://github.com/input-output-hk/hedgehog-extras/blob/c1130836a55f662b17d2e86daae04362d65655e1/src/Hedgehog/Extras/Test/Process.hs#L305

carbolymer · 2025-11-07T11:08:28Z

@Jimbo4350

So we should change to liftIOAnnotated as we can tell where the exception was thrown from.

Yes, seems that evalIO eats up the call stack, which is sad. However, all the annotations from note_ are gone with liftIOAnnotated which is also a problem.

However we can have a cake and eat it too. See my commit Fix liftToIntegration to report the exception location to Hedgehog.

Small thing, that the exception type reported is StringException and UnliftIO.Exception.throwString called with: is a bit confusing. Additionally, if the caught exception carries a call stack, it will be rendered too leading to printing of 3 call stacks like here:

I guess it is fine, since this points to both location of the error there and liftToIntegration.

carbolymer · 2025-11-07T13:44:27Z

cardano-testnet/src/Testnet/Runtime.hs

+
+
+-- | Runs an action in background, and registers its cancellation to 'MonadResource'.
+asyncRegister_ :: HasCallStack


I think this can be upstreamed to hedgehog-extras too. In the end I think there is hardly a benefit of using evalM there which requires MonadTest constraint: https://github.com/input-output-hk/hedgehog-extras/blob/c1130836a55f662b17d2e86daae04362d65655e1/src/Hedgehog/Extras/Test/Concurrent.hs#L97
I'm not sure what would have happened to make async throw right away (resource exhaustion perhaps, but that would signal larger problems anyway).

carbolymer · 2025-11-07T23:29:37Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+liftToIntegration :: (HasCallStack, MonadCatch m, MonadResource m, MonadTest m)
+                  => (forall n. (MonadCatch n, MonadResource n, MonadUnliftIO n, MonadFail n) => n a)
+                  -> m a
+liftToIntegration act =  do
+   internalState <- liftResourceT getInternalState
+   catch @_ @SomeException (liftIO $ runInternalState act internalState) (withFrozenCallStack $ failException . toException . stringException . displayException)


I have generalised liftToIntegration in the latest commit Generalise liftToIntegration so it's not tied to RIO or Integration. This way we could add it to hedgehog-extras together.

Small caveat: one has to foresee what constraints for act are needed upfront, otherwise GHC will not be happy.

We could use now a better (shorter preferably) name now.

carbolymer · 2025-11-07T23:30:15Z

cardano-testnet/src/Parsers/Run.hs

+        runRIO () $ createTestnetEnv
+                   testnetOptions genesisOptions def
+                   conf
+        withResourceMap (\rm -> void . runRIO rm $ cardanoTestnet testnetOptions conf)


What's the advantage over:

Suggested change

withResourceMap (\rm -> void . runRIO rm $ cardanoTestnet testnetOptions conf)

void . runResourceT $ cardanoTestnet testnetOptions conf

?

Jimbo4350 force-pushed the jordan/remove-integration-monad branch 3 times, most recently from b99bb37 to 0bf6d79 Compare October 22, 2025 13:55

Jimbo4350 force-pushed the jordan/remove-integration-monad branch 19 times, most recently from 0f61855 to d7b60ca Compare November 3, 2025 15:26

Jimbo4350 marked this pull request as ready for review November 4, 2025 12:08

Jimbo4350 requested a review from a team as a code owner November 4, 2025 12:08

Jimbo4350 changed the title ~~Remove Integration Monad~~ Do not merge: Remove Integration Monad Nov 4, 2025

carbolymer reviewed Nov 4, 2025

View reviewed changes

cardano-testnet/src/Testnet/Orphans.hs Outdated Show resolved Hide resolved

carbolymer reviewed Nov 4, 2025

View reviewed changes

carbolymer requested changes Nov 4, 2025

View reviewed changes

cardano-testnet/src/Testnet/Runtime.hs Outdated Show resolved Hide resolved

carbolymer reviewed Nov 4, 2025

View reviewed changes

cardano-testnet/src/Testnet/Runtime.hs Show resolved Hide resolved

carbolymer reviewed Nov 5, 2025

View reviewed changes

Jimbo4350 added 8 commits November 6, 2025 10:02

Implement asyncRegister_

e86447d

This differs from hedgehog-extras's asyncRegister function in that the thread is linked before it is cancelled.

Add hprop_asyncRegister_sanity_check

5b970e8

Remove Integration monad return type from createTestnetEnv and

3e6b8a4

cardanoTestnet

Remove MonadTest constraint from createSPOGenesisAndFiles

8270a2b

Remove MonadTest constraint from startNode and startLedgerNewEpochSta…

37ade3d

…teLogging

Remove MonadTest constraints from assertion functions

8e04ac0

Propagate liftToIntegration and liftIOAnnotated

d1006cc

Fix build issues

b8c8b53

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from 568a7ac to 9709995 Compare November 6, 2025 14:02

carbolymer requested changes Nov 6, 2025

View reviewed changes

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from 9709995 to c0c3365 Compare November 6, 2025 16:52

Review changes

1333cdc

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from c0c3365 to 4a2c1dc Compare November 6, 2025 18:30

Jimbo4350 requested a review from a team as a code owner November 6, 2025 18:30

Replace liftIO with liftIOAnnotated

6806869

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from 4a2c1dc to 6806869 Compare November 6, 2025 19:16

Jimbo4350 requested a review from a team as a code owner November 6, 2025 19:21

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from ffcf2d2 to 6806869 Compare November 6, 2025 20:52

carbolymer force-pushed the jordan/remove-integration-monad branch 2 times, most recently from 44b4f2f to b31495b Compare November 7, 2025 11:40

Fix infinite loop

81be8a4

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from b31495b to 2e4b895 Compare November 7, 2025 12:39

Bump iohkNix and hackageNix

43697cc

Jimbo4350 force-pushed the jordan/remove-integration-monad branch from 2e4b895 to 43697cc Compare November 7, 2025 12:42

Fix liftToIntegration to report the exception location to Hedgehog

1d5b8a9

carbolymer reviewed Nov 7, 2025

View reviewed changes

Jimbo4350 mentioned this pull request Nov 7, 2025

Expose process related functions without a MonadTest constraint input-output-hk/hedgehog-extras#107

Open

Generalise liftToIntegration

a8f6270

carbolymer reviewed Nov 7, 2025

View reviewed changes



		-- \| Runs an action in background, and registers its cancellation to 'MonadResource'.
		asyncRegister_ :: HasCallStack

	withResourceMap (\rm -> void . runRIO rm $ cardanoTestnet testnetOptions conf)
	void . runResourceT $ cardanoTestnet testnetOptions conf

Do not merge: Remove Integration Monad #6346

Are you sure you want to change the base?

Do not merge: Remove Integration Monad #6346

Conversation

Jimbo4350 commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Note on CI

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carbolymer commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jimbo4350 commented Oct 21, 2025 •

edited

Loading

carbolymer commented Nov 7, 2025 •

edited

Loading