Merge pull request #14397 from MinaProtocol/fix/itn-mangled-errors

Fix/itn mangled errors

Author: deepthiskumar

src/lib/gossip_net/libp2p.ml

@@ -217,7 +217,7 @@ module Make (Rpc_intf : Network_peer.Rpc_intf.Rpc_interface_intf) :
         | Mina_net2.Libp2p_helper_died_unexpectedly ->
             on_unexpected_termination ()
         | _ ->
-            raise exn
+            Exn.reraise exn "Mina_net2 raised an exception"
       in
       let%bind seeds_from_url =
         match config.seed_peer_list_url with

src/lib/o1trace/o1trace.ml

@@ -86,9 +86,9 @@ let thread name f =
       let ctx = with_o1trace ~name ctx in
       match Scheduler.within_context ctx f with
       | Error () ->
-          failwithf
-            "timing task `%s` failed, exception reported to parent monitor" name
-            ()
+          (* Scheduler.within_context will send the actual error to the parent monitor asynchronously.
+           * At this point, the thread has crashed, so we just return a Deferred that will never resolve *)
+          Deferred.create (Fn.const ())
       | Ok x ->
           x )
 
@@ -102,11 +102,12 @@ let sync_thread name f =
       let start_time = Time_ns.now () in
       let ctx = Scheduler.current_execution_context () in
       let ctx = with_o1trace ~name ctx in
-      match Scheduler.within_context ctx f with
-      | Error () ->
-          failwithf
-            "sync timing task `%s` failed, exception reported to parent monitor"
-            name ()
+      match
+        Scheduler.Private.with_execution_context (Scheduler.Private.t ()) ctx
+          ~f:(fun () -> Result.try_with f)
+      with
+      | Error exn ->
+          Exn.reraise exn "exception caught by O1trace.sync_thread"
       | Ok result ->
           let elapsed_time = Time_ns.abs_diff (Time_ns.now ()) start_time in
           on_job_exit' fiber elapsed_time ;
@@ -125,6 +126,12 @@ let () =
 
 let%test_module "thread tests" =
   ( module struct
+    exception Test_exn
+
+    let is_test_exn exn =
+      (* there isn't a great way to compare the exn to the one that was thrown due to how async mangles the exn, so we do this instead *)
+      String.is_substring (Exn.to_string exn) ~substring:"(Test_exn)"
+
     let child_of n =
       match
         let prev_sync_fiber = !current_sync_fiber in
@@ -214,5 +221,31 @@ let%test_module "thread tests" =
                              Deferred.unit ) ) ) ;
                   Deferred.unit ) ) )
 
+    let%test_unit "exceptions are handled properly when raised in first cycle \
+                   of a thread" =
+      test (fun stop ->
+          match%map
+            Monitor.try_with (fun () ->
+                thread "test" (fun () -> raise Test_exn) )
+          with
+          | Ok _ ->
+              failwith "expected a failure"
+          | Error exn ->
+              assert (is_test_exn exn) ;
+              stop () )
+
+    let%test_unit "exceptions are handled properly when raised in first cycle \
+                   of a sync_thread" =
+      test (fun stop ->
+          match%map
+            Monitor.try_with (fun () ->
+                sync_thread "test" (fun () -> raise Test_exn) )
+          with
+          | Ok _ ->
+              failwith "expected a failure"
+          | Error exn ->
+              assert (is_test_exn exn) ;
+              stop () )
+
     (* TODO: recursion tests *)
   end )