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

Thread/Fiber/Kernel#raise is incorrectly labeled #1667

Open
Forthoney opened this issue Dec 6, 2023 · 3 comments
Open

Thread/Fiber/Kernel#raise is incorrectly labeled #1667

Forthoney opened this issue Dec 6, 2023 · 3 comments

Comments

@Forthoney
Copy link
Contributor

Forthoney commented Dec 6, 2023

Small issues

Thread#raise is incorrectly written as def self?.raise when it should just be def raise

Big issues

More fundamentally, the method signature for Kernel/Thread/Fiber#raise is flawed, even on the official documentation. I am unsure if the current (Ruby 3.2.2) behavior is the intended one, or if the documentation is the intended one.

To my surprise, raise very interesting & complex behavior. Specifically, these are the rules I found for raise on Fiber, and Thread via experimentation (the official documentation is also not quite correct unfortunately). They may not be the only rules though.

def raise: () -> nil
         | (String message) -> nil
         | (Exception ex) -> nil # (edit 5) change from Exception class to Exception instance
         | (_Exception ex) -> nil
         | (_Exception ex, _ToS message) -> nil # (1) change from String to _ToS
         | (_Exception ex, _ToS message, String backtrace) -> nil # (2) change from Array[String] to String
         | (_Exception ex, _ToS message, Array[String] backtrace) -> nil # (3) change from String to _ToS
         | (_Exception, **kwargs) -> nil # (4) new rule

For Kernel, the rules are the same except it also accepts an optional keyword argument ?cause: Exception at every rule variant. Note that cause: is not _Exception but Exception - _Exception is a class interface, Exception is an instance of that interface.

Here are the explanations behind each change

  1. Any object that can be to_s will be accepted as a valid message
  2. If only one String argument is provided for backtrace, it will treat it like an array of one element
  3. Same as 1
  4. All keyword arguments (except :cause in Kernel) will be converted to a Hash, stringified, then used as the message

I do not know if #raise was intended to have such complicated behavior and the docs were not updated or if this is a implementation bug. The inconsistent behavior between message and backtrace and the implicit Hashification of kwargs leads me to believe that this is a bug. If a core committer can confirm this is not intentional, I would be happy to submit an issue to the bug tracker. There perhaps we can have a more productive conversation about whether to fix this or keep it as is an update the docs.

EDIT: Just found another possible signature (5)

@soutaro
Copy link
Member

soutaro commented Dec 7, 2023

@Forthoney Thank you for reporting the issue. It makes sense.

(4) might be unnecessary. foo(RuntimeError, foo: 1) is equivalent to passing a Hash (foo(RuntimeError, { foo: 1 })) that has to_s method.

@Forthoney
Copy link
Contributor Author

Would you say ruby-doc.org also needs an update?

@soutaro
Copy link
Member

soutaro commented Dec 8, 2023

I don't think we should update the RDoc about this, but what update do you have in your mind exactly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants