From f640ea97e4027f250d0512c1983f9b8f5561900a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 17 Sep 2024 15:46:11 +0200 Subject: [PATCH] Improve exception handling --- obuilder | 2 +- ocurrent-plugin/connection.ml | 7 +++---- test/test_plugin.ml | 4 ++-- worker/cluster_worker.ml | 4 ++-- worker/context.ml | 8 ++++---- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/obuilder b/obuilder index d21e5bf2..bc6eca86 160000 --- a/obuilder +++ b/obuilder @@ -1 +1 @@ -Subproject commit d21e5bf2e05762e02e3f34b82365c514634e02ad +Subproject commit bc6eca86e5624b32ca58fb0b5e43e7e15f188f58 diff --git a/ocurrent-plugin/connection.ml b/ocurrent-plugin/connection.ml index 9a23d327..cbdf136b 100644 --- a/ocurrent-plugin/connection.ml +++ b/ocurrent-plugin/connection.ml @@ -126,9 +126,8 @@ let submit ~job ~pool ~action ~cache_hint ?src ?secrets ~urgent t ~priority ~swi | Lwt.Canceled as ex -> if !stage = `Rate_limit then Prometheus.Gauge.dec_one Metrics.queue_rate_limit else Log.warn (fun f -> f "Cancelled at unexpected point!"); - Lwt.fail ex - | ex -> - Lwt.fail ex + Lwt.reraise ex + | ex -> Lwt.reraise ex ) in limiter_thread := Some use_thread; @@ -139,7 +138,7 @@ let submit ~job ~pool ~action ~cache_hint ?src ?secrets ~urgent t ~priority ~swi Lwt.pause () >>= fun () -> if Capability.problem sched = None then ( (* The job failed but we're still connected to the scheduler. Report the error. *) - Lwt.fail_with (Fmt.str "%a" Capnp_rpc.Exception.pp err) + Fmt.failwith "%a" Capnp_rpc.Exception.pp err ) else ( limiter_thread := None; begin match !stage with diff --git a/test/test_plugin.ml b/test/test_plugin.ml index 94ef4fc0..80764630 100644 --- a/test/test_plugin.ml +++ b/test/test_plugin.ml @@ -200,7 +200,7 @@ let cancel_rate_limit () = (fun _ -> failwith "Should have failed!") (function | Lwt.Canceled -> Lwt.return_unit - | ex -> Lwt.fail ex) + | ex -> Lwt.reraise ex) >>= fun () -> (* Finish connecting to the scheduler. *) let sched = @@ -221,7 +221,7 @@ let cancel_rate_limit () = (fun _ -> failwith "Should have failed!") (function | Lwt.Canceled -> Lwt.return_unit - | ex -> Lwt.fail ex) + | ex -> Lwt.reraise ex) >>= fun () -> Lwt.return_unit diff --git a/worker/cluster_worker.ml b/worker/cluster_worker.ml index 0e1df94f..1b71e774 100644 --- a/worker/cluster_worker.ml +++ b/worker/cluster_worker.ml @@ -553,7 +553,7 @@ let run ?switch ?build ?(allow_push=[]) ?(healthcheck_period = 600.0) ?prune_thr | _, Some switch when not (Lwt_switch.is_on switch) -> Lwt.return `Cancelled | Some problem, _ -> Log.info (fun f -> f "Worker loop failed (probably because queue connection failed): %a" Fmt.exn ex); - Lwt.fail (Failure (Fmt.to_to_string Capnp_rpc.Exception.pp problem)) (* Will retry *) + Fmt.failwith "%a" Capnp_rpc.Exception.pp problem (* Will retry *) | None, _ -> Lwt.return (`Crash ex) ) @@ -566,6 +566,6 @@ let run ?switch ?build ?(allow_push=[]) ?(healthcheck_period = 600.0) ?prune_thr in reconnect () >>= function | `Cancelled -> Lwt.return_unit - | `Crash ex -> Lwt.fail ex + | `Crash ex -> Lwt.reraise ex module Obuilder_config = Obuilder_build.Config diff --git a/worker/context.ml b/worker/context.ml index 73c9fd40..76bdac4b 100644 --- a/worker/context.ml +++ b/worker/context.ml @@ -97,7 +97,7 @@ let win32_unlink fn = Lwt.catch (fun () -> Lwt_unix.unlink fn) (function - | Unix.Unix_error (Unix.EACCES, _, _) as exn -> + | Unix.Unix_error (Unix.EACCES, _, _) as ex -> (* Try removing the read-only attribute before retrying unlink. We catch any exception here and ignore it in favour of the original [exn]. *) Lwt.catch @@ -110,10 +110,10 @@ let win32_unlink fn = (* If everything succeeded but the final removal still failed, restore original permissions *) Lwt_unix.chmod fn st_perm >>= fun () -> - Lwt.fail exn) + Lwt.reraise ex) ) - (fun _ -> Lwt.fail exn) - | exn -> Lwt.fail exn) + (fun _ -> Lwt.reraise ex) + | ex -> Lwt.reraise ex) let unlink = if Sys.win32 then