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

Add protect function, as alternative to missing try finally construct #155

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

glennsl
Copy link
Contributor

@glennsl glennsl commented Oct 12, 2023

As pointed out in this SO question, there is no built-in try finally construct, which is confusing to people coming from JS. protect has existed in OCaml since 4.08, so figured it ought to exist here as well.

Not sure if it should be in RescriptCore though, or perhaps in Error instead?

@glennsl glennsl changed the title Add protect function, as alternative to missing try finnaly construct Add protect function, as alternative to missing try finally construct Oct 12, 2023
@Sod-Almighty
Copy link

It would make more sense to just add the missing finally keyword. This is a real language with a real parser, right guys? Not just a fragile shim over OCaml?

@zth
Copy link
Collaborator

zth commented Oct 25, 2023

Sorry, missed this! Will try and get back to this in the coming days. It's true we miss a solid way of emulating finally now, and we should definitively solve that in some (good) way.

@zth
Copy link
Collaborator

zth commented Oct 26, 2023

@glennsl in my mind this is a good solution. There's of course the option to bake syntax sugar for this into the language itself as well with a proper finally keyword etc like @Sod-Almighty mentions above, but I'm not sure it's worth the added complexity, adding a new keyword, etc. We can think about it some more, and adding this doesn't exclude possibly adding that in the future.

@Sod-Almighty no idea if you're trying to be funny with your strangely worded message or how to interpret it.

@cristianoc
Copy link
Contributor

As a side comment, extending the language with a finally clause would require actual design (it's not just a keyword). Including what the format of the clause looks like, to how it is translated, to how the code is generated, corner cases, perf etc.

@zth
Copy link
Collaborator

zth commented Oct 27, 2023

@glennsl I say we go forward with this. I wonder about the naming though. Can we come up with something more illustrative, now that we're going for familiarity for JS devs rather than OCaml? I don't have any good ideas myself yet, do you have any off the top of your head?

@Sod-Almighty
Copy link

@Sod-Almighty no idea if you're trying to be funny with your strangely worded message or how to interpret it.

In another thread I asked about finally in ReScript, and the responses strongly implied that ReScript's language features were "limited" to concepts supported by OCaml, due to being implemented in it.

If ReScript is a real language with a real parser – written in OCaml or whatever else – and compiles to JS rather than to OCaml, then OCaml's lack of a finally keyword is completely irrelevant. So either the responses I received were incorrect, or something fishy is going on with ReScript.

@cristianoc
Copy link
Contributor

@Sod-Almighty no idea if you're trying to be funny with your strangely worded message or how to interpret it.

In another thread I asked about finally in ReScript, and the responses strongly implied that ReScript's language features were "limited" to concepts supported by OCaml, due to being implemented in it.

If ReScript is a real language with a real parser – written in OCaml or whatever else – and compiles to JS rather than to OCaml, then OCaml's lack of a finally keyword is completely irrelevant. So either the responses I received were incorrect, or something fishy is going on with ReScript

Feel free to come up with competent comments next time.

@Sod-Almighty
Copy link

Feel free to come up with competent comments next time.

Feel free to stop being snide.

@glennsl
Copy link
Contributor Author

glennsl commented Oct 30, 2023

@glennsl I say we go forward with this. I wonder about the naming though. Can we come up with something more illustrative, now that we're going for familiarity for JS devs rather than OCaml? I don't have any good ideas myself yet, do you have any off the top of your head?

tryWith maybe?

@zth
Copy link
Collaborator

zth commented Oct 30, 2023

@glennsl I like tryWith 👍

@glennsl
Copy link
Contributor Author

glennsl commented Oct 30, 2023

tryWith maybe?

Looking at the example, I don't think tryWith will read very well unfortunately:

  try protect(~finally=() => Js.log("finally"), () => failwith("oh no!")) catch {
  | Failure(err) => Js.log(err)
  }

guard is another option, but hardly better than protect.

@zth
Copy link
Collaborator

zth commented Nov 3, 2023

withFinally? That reads pretty well to me in the example:

try withFinally(
  () => failwith("oh no!"),
  ~finally=() => Js.log("finally"),
) catch {
| Failure(err) => Js.log(err)
}

I guess I'm not super set on guard nor protect because they're not actually doing any of that, right. It's just about running the finally function regardless, it's not really guarding or protecting from exceptions. Which is how I interpret the name, but that might just be me.

@DZakh
Copy link
Contributor

DZakh commented Nov 4, 2023

I like the withFinally name

@glennsl
Copy link
Contributor Author

glennsl commented Nov 5, 2023

So there are two major use cases for this that I can see.

One is where you just want to make sure that some code is run whether or not an exception is raised, but you don't want to handle the exception immediately. This is usually in order to clean up some resource, and in that case I think protect or guard fits the description. withFinally works less well I think.

The other is where you want to also handle raised exceptions immediately, in which case you need to use it with the try construct for its pattern matching. withFinally works well here, despite the redundancy of finally being used twice.

Seems hard to find a good name that covers both scenarios well.

@Sod-Almighty
Copy link

How about ensure?

(Sorry was that a "competent comment"? It's so hard to tell.)

@DZakh
Copy link
Contributor

DZakh commented Nov 6, 2023

For me, ensure, protect, and guard don't say anything. Well, guard has association with TS's type guards, but that's way off.

@zth
Copy link
Collaborator

zth commented Nov 6, 2023

@Sod-Almighty I suggest you drop the attitude right now if you hope to ever be taken seriously. You have your chance now to drop it and move on.

Yeah definitively tricky. @glennsl makes a good point with the two use cases and how they differ in naming. I like the ensure suggestion because to me it communicates, albeit a bit vaguely, that we're ensuring that something runs (finally). "Finally" doesn't go that well with "ensure" though, but maybe we don't have to call the function "finally" either, if we stray far enough away from the try/catch/finally concept.

Sounds like we need to mull on this a bit more.

@Sod-Almighty
Copy link

You guys started it by mocking me when all I did was ask a question. In any case, I got my answer: unfinished language and toxic devs. I'm out.

Comment on lines +73 to +86
`protect(~finally, f)`

Tries to execute the function `f`, and ensures that `finally` will be called
whether `f` raises an exception or not.

Any exception raised by `f` will be re-raised in order to be handled by the
user. If `finally` raises, then that exception will be emitted instead, and
any exception raised by `f` will go unnoticed.

```res example
try protect(~finally=() => Js.log("finally"), () => failwith("oh no!")) catch {
| Failure(err) => Js.log(err)
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glenn, just a little formatting

Suggested change
`protect(~finally, f)`
Tries to execute the function `f`, and ensures that `finally` will be called
whether `f` raises an exception or not.
Any exception raised by `f` will be re-raised in order to be handled by the
user. If `finally` raises, then that exception will be emitted instead, and
any exception raised by `f` will go unnoticed.
```res example
try protect(~finally=() => Js.log("finally"), () => failwith("oh no!")) catch {
| Failure(err) => Js.log(err)
}
```
`protect(~finally, f)` tries to execute the function `f`, and ensures that
`finally` will be called whether `f` raises an exception or not.
Any exception raised by `f` will be re-raised in order to be handled by the
user. If `finally` raises, then that exception will be emitted instead, and
any exception raised by `f` will go unnoticed.
## Examples
```rescript
try protect(~finally=() => Console.log("finally"), () => panic("oh no!")) catch {
| Failure(err) => Console.log(err)
}

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.

6 participants