Skip to content

Commit b5e9a3a

Browse files
authored
message-forwrding: keep backtraces when reraising an exception (#6995)
A few functions in message_forwarding need to do some cleanup when encountering an exception, but in doing so, the original backtrace is lost. This makes it difficult to track down the original location that raised the exception. I've considered other options instead of calling Backtrace.is_important: - Call `Backtrace.reraise e e`: this is more expensive than it needs to be. - Create a new function that can be easily used: This is awkward to backport, and in my attempts the resulting code was not great to read. So I ended up with the compromise of using Backtrace.is_important, which does exactly what's needed here, even though it's difficult to enforce correctly across the codebase.
2 parents f2a146c + 1268943 commit b5e9a3a

1 file changed

Lines changed: 9 additions & 3 deletions

File tree

ocaml/xapi/message_forwarding.ml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ let iter_with_drop ?(doc = "performing unknown operation") f xs =
208208
let log_exn ?(doc = "performing unknown operation") f x =
209209
try f x
210210
with e ->
211+
Backtrace.is_important e ;
211212
debug "Caught exception while %s in message forwarder: %s" doc
212213
(ExnHelper.string_of_exn e) ;
213214
raise e
@@ -332,9 +333,10 @@ functor
332333
let tolerate_connection_loss fn success timeout =
333334
try fn ()
334335
with
335-
| Api_errors.Server_error (ercode, params)
336+
| Api_errors.Server_error (ercode, _) as e
336337
when ercode = Api_errors.cannot_contact_host
337338
->
339+
Backtrace.is_important e ;
338340
debug
339341
"Lost connection with slave during call (expected). Waiting for \
340342
slave to come up again." ;
@@ -346,8 +348,7 @@ functor
346348
let rec poll i =
347349
match i with
348350
| 0 ->
349-
raise (Api_errors.Server_error (ercode, params))
350-
(* give up and re-raise exn *)
351+
raise e (* give up and re-raise exn *)
351352
| i -> (
352353
match success () with
353354
| Some result ->
@@ -1311,6 +1312,7 @@ functor
13111312
vbds ;
13121313
vbds
13131314
with e ->
1315+
Backtrace.is_important e ;
13141316
debug "Caught exception marking VBD for %s on VM %s: %s" doc
13151317
(Ref.string_of vm)
13161318
(ExnHelper.string_of_exn e) ;
@@ -1492,6 +1494,7 @@ functor
14921494
(Helpers.will_have_qemu ~__context ~self:vm) ;
14931495
Xapi_network_sriov_helpers.reserve_sriov_vfs ~__context ~host ~vm
14941496
with e ->
1497+
Backtrace.is_important e ;
14951498
clear_vif_reservations ~__context ~vm ;
14961499
clear_reservations ~__context ~vm ;
14971500
raise e
@@ -1636,6 +1639,7 @@ functor
16361639
) ;
16371640
try f ()
16381641
with exn ->
1642+
Backtrace.is_important exn ;
16391643
if !restore_old_values_on_error then (
16401644
Db.VM.set_memory_dynamic_min ~__context ~self:vm
16411645
~value:old_dynamic_min ;
@@ -5215,6 +5219,7 @@ functor
52155219
(fun (vdi, op) -> mark_vdi ~__context ~vdi ~doc ~op)
52165220
vdi
52175221
with e ->
5222+
Backtrace.is_important e ;
52185223
Option.iter
52195224
(fun (sr, op) -> SR.unmark_sr ~__context ~sr ~doc ~op)
52205225
sr ;
@@ -6564,6 +6569,7 @@ functor
65646569
-> (
65656570
match rest with
65666571
| [] ->
6572+
Backtrace.is_important e ;
65676573
debug
65686574
"Ran out of hosts to try (and no cluster host on \
65696575
ourselves), reporting error" ;

0 commit comments

Comments
 (0)