Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise and reraise exceptions with Stdlib rather than Lwt. #430

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

MisterDA
Copy link
Contributor

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 More raise less fail #1008.

MisterDA added 3 commits July 22, 2024 15:22
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.
@MisterDA MisterDA changed the title Raise and reraise exceptions with Stdlib rather that Lwt. Raise and reraise exceptions with Stdlib rather than Lwt. Jul 22, 2024
@hannesm
Copy link
Member

hannesm commented Aug 29, 2024

Thanks for your contribution. I merged the main branch into your PR, and will merge this now.

@hannesm hannesm merged commit 18b8491 into mirage:main Aug 29, 2024
0 of 8 checks passed
hannesm added a commit to hannesm/opam-repository that referenced this pull request Aug 29, 2024
CHANGES:

* adapt to TLS 1.0.0 API changes, bump OCaml lower bound to 4.13 (mirage/ocaml-conduit#432 @hannesm)
* conduit-lwt-unix: improve the no TLS error message (mirage/ocaml-conduit#431 @filipeom)
* Remove 4.12 CI runners, add 5.2 (mirage/ocaml-conduit#433 @art-w)
* Update GitHub actions (mirage/ocaml-conduit#429 @smorimoto)
* use failwith instead of Lwt.fail_with, use Lwt.reraise (mirage/ocaml-conduit#430 @MisterDA)
* switch to sexplib0 instead of sexplib for lighter dependencies (mirage/ocaml-conduit#427 @emillon)
@MisterDA MisterDA deleted the lwt-exceptions branch August 29, 2024 10:13
hannesm added a commit to hannesm/opam-repository that referenced this pull request Aug 30, 2024
CHANGES:

* adapt to TLS 1.0.0 API changes, bump OCaml lower bound to 4.13 (mirage/ocaml-conduit#432 @hannesm)
* conduit-lwt-unix: improve the no TLS error message (mirage/ocaml-conduit#431 @filipeom)
* Remove 4.12 CI runners, add 5.2 (mirage/ocaml-conduit#433 @art-w)
* Update GitHub actions (mirage/ocaml-conduit#429 @smorimoto)
* use failwith instead of Lwt.fail_with, use Lwt.reraise (mirage/ocaml-conduit#430 @MisterDA)
* switch to sexplib0 instead of sexplib for lighter dependencies (mirage/ocaml-conduit#427 @emillon)
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

* adapt to TLS 1.0.0 API changes, bump OCaml lower bound to 4.13 (mirage/ocaml-conduit#432 @hannesm)
* conduit-lwt-unix: improve the no TLS error message (mirage/ocaml-conduit#431 @filipeom)
* Remove 4.12 CI runners, add 5.2 (mirage/ocaml-conduit#433 @art-w)
* Update GitHub actions (mirage/ocaml-conduit#429 @smorimoto)
* use failwith instead of Lwt.fail_with, use Lwt.reraise (mirage/ocaml-conduit#430 @MisterDA)
* switch to sexplib0 instead of sexplib for lighter dependencies (mirage/ocaml-conduit#427 @emillon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants