ocurrent / obuilder

Experimental "docker build" alternative using btrfs/zfs snapshots
Apache License 2.0
60 stars 17 forks source link

Raise and reraise exceptions with Stdlib rather than Lwt #188

Closed MisterDA closed 2 months ago

MisterDA commented 2 months ago

Lwt's documentation reads:

In most cases, it is better to use failwith s from the standard library.

and

Whenever possible, it is recommended to use raise exn instead, as raise captures a backtrace, while Lwt.fail does not. If you call raise exn in a callback that is expected by Lwt to return a promise, Lwt will automatically wrap exn in a rejected promise, but the backtrace will have been recorded by the OCaml runtime.

For example, bind's second argument is a callback which returns a promise. And so it is recommended to use raise in the body of that callback.

Use Lwt.fail only when you specifically want to create a rejected promise, to pass to another function, or store in a data structure.

Prefer to capture backtraces to improve debugability.

See also https://github.com/ocsigen/lwt/pull/1008.

MisterDA commented 2 months ago

@shonfeder I can't merge this because of the new rules preventing merges if the CI doesn't pass.

shonfeder commented 2 months ago

@shonfeder I can't merge this because of the new rules preventing merges if the CI doesn't pass.

The system works! :D Tho unfortunately the CI failures here are just https://github.com/ocurrent/obuilder/issues/186.

shonfeder commented 2 months ago

Thanks again for this fix :D