Skip to content
New issue

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

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

Already on GitHub? # to your account

Add process names, refocus scope, rework API #71

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Add process names, refocus scope, rework API #71

wants to merge 27 commits into from

Conversation

lpil
Copy link
Member

@lpil lpil commented Feb 13, 2025

Reworking the API of this package for a v1 release! The big new feature is named processes, which will make writing OTP programs much easier in Gleam.

@lpil lpil marked this pull request as ready for review February 26, 2025 14:03
@bcpeinhardt
Copy link

General note: There's a lot of really good QOL stuff happening in this PR :)

@bcpeinhardt
Copy link

Made a small PR with some minor docs changes so you don't have to bother copying them over: #72

Co-authored-by: Giacomo Cavalieri <giacomo.cavalieri@icloud.com>
Copy link

@rawhat rawhat left a comment

Choose a reason for hiding this comment

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

i like these changes. RIP rescue 🙏🏻

@lpil lpil mentioned this pull request Feb 28, 2025
@sbergen
Copy link

sbergen commented Mar 1, 2025

Since we're introducing breaking changes, would it make sense to include #67 also? It contains changes from String to Dynamic, and Dynamic to ExitReason.

I can rebase it on top of this branch, if it helps.

@lpil
Copy link
Member Author

lpil commented Mar 1, 2025

Good idea @sbergen! I wonder if there's any further improvements we could make to those types...

@sbergen
Copy link

sbergen commented Mar 1, 2025

Good idea @sbergen! I wonder if there's any further improvements we could make to those types...

I still find the asymmetry of send_abnormal_exit using String somewhat complicated and restrictive. Would rather have a more symmetric send_exit(Pid, ExitReason). Being more symmetric makes things easier to think about (for me, at least), and being able to send a decodable value as an exit reason can be useful. If we also added exit(ExitReason) we'd be more in line with the Erlang exit/1 and exit/2 semantics, as far as I can see.

@lpil
Copy link
Member Author

lpil commented Mar 3, 2025

What are some uses you have for decoding the exit reason?

If we also added exit(ExitReason) we'd be more in line with the Erlang exit/1 and exit/2 semantics, as far as I can see.

This would mean we have it completely unchecked by the type system, and the programmer just have to get the correct magic values!

@sbergen
Copy link

sbergen commented Mar 3, 2025

What are some uses you have for decoding the exit reason?

The uses I've had would be to report a more structured error from a process that should terminate. E.g. when using a process to communicate with a server, where the protocol specifies that the connection should be immediately closed on any protocol violations, it would be nice to be able to report if the reason to terminate was due to a) a protocol violation from the server, or b) one of our own assertions about situations that shouldn't happen (e.g. Erlang timers often lead me to add assertions like this).

You could argue that these could/should be reported through a Subject, but if you need to be monitoring a process already, that gets much more complicated than just trying to decode a value IME.

If we also added exit(ExitReason) we'd be more in line with the Erlang exit/1 and exit/2 semantics, as far as I can see.

This would mean we have it completely unchecked by the type system, and the programmer just have to get the correct magic values!

I know, but as I pointed out in #67, the string in Abnormal(String) could be just about anything right now also (which is why Abnormal(Dynamic) makes more sense). I don't think there's anything we can do about processes being able to panic with an unexpected value: if not from Gleam code with e.g. let assert (I proposed making GleamError a type you could decode to also, but that suggestion wasn't popular), then from Erlang code causing the process to error/exit.

I guess some kind of API where you could have typed processes, Pid(a), and ExitReason(a) with some extra variant could perhaps be made to work, but that would be a much bigger change, and I somehow feel it would be more trouble than it would be worth.

@lpil
Copy link
Member Author

lpil commented Mar 4, 2025

It sounds like a mistake to me to be using non-local error handling in such a fine-grained way.

I know, but as I pointed out in #67, the string in Abnormal(String) could be just about anything right now also (which is why Abnormal(Dynamic) makes more sense). I don't think there's anything we can do about processes being able to panic with an unexpected value: if not from Gleam code with e.g. let assert (I proposed making GleamError a type you could decode to also, but that suggestion wasn't popular), then from Erlang code causing the process to error/exit.

The message is a debug convenience rather than a tool for exception based flow control, the same as the text message one can give to a panic.

Something being unstructured is not an argument in favour of removing useful structure from a related thing. The Erlang design is informative but replicating it is not a goal as we are focusing on types and making invalid use impossible, while the Erlang API attempts neither.

I guess some kind of API where you could have typed processes, Pid(a), and ExitReason(a) with some extra variant could perhaps be made to work, but that would be a much bigger change, and I somehow feel it would be more trouble than it would be worth.

Pids are not typable with their messages as it makes call impossible to implement, and exits are not typable as they can originate from outside of Gleam, just like exceptions.

Now is the time to make large changes! There will be no other time to do so.

@lpil
Copy link
Member Author

lpil commented Mar 5, 2025

OK, I am convinced @sbergen ! I have merged your changes into this branch. :)

@sbergen
Copy link

sbergen commented Mar 5, 2025

It sounds like a mistake to me to be using non-local error handling in such a fine-grained way.

As a package author, I was thinking of it more as a HTTP 500 (oops, my bad) vs 502 (I can't do my job because of a third party) that could be reported to the user of the package...

The message is a debug convenience rather than a tool for exception based flow control, the same as the text message one can give to a panic.

...but indeed, this sort of information could be included in strings also. I just like types a lot more than strings :)

...and exits are not typable as they can originate from outside of Gleam, just like exceptions.

I think you could have one typed variant of ExitReason, but especially if we don't want to encourage using exit values, I don't think it's really an avenue worth pursuing.

OK, I am convinced @sbergen ! I have merged your changes into this branch. :)

send_abnormal_exit still takes a string, but I think as long as Abnormal holds Dynamic, it will make using dynamic exit values with FFI for those who really want to do it easy enough 👍

lpil and others added 2 commits March 13, 2025 12:40
Co-authored-by: Sakari Bergen <s@beatwaves.net>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants