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

Allow suppressing Errno::ECHILD without triggering Lint/HandleExceptions #8141

Closed
tlubz opened this issue Jun 11, 2020 · 3 comments
Closed

Comments

@tlubz
Copy link

tlubz commented Jun 11, 2020

Is your feature request related to a problem? Please describe.

When using Process.wait(pid) to await a child process exiting, it's expected to raise Errno::ECHILD if the process has already terminated. The appropriate response in that case is usually to rescue and proceed without action.

Additionally, there's not an effective way to avoid this error being raised. There's not a simple, portable way to check that a process is not running. Even if there was, there is a race, where the child may exit between checking and calling wait.

Thus in multiprocessing code, you frequently see code like the following:

begin
  Process.wait(pid)
rescue Errno::ECHILD
  # this is expected, the child process has terminated before calling wait
end

This generally requires ugly code like the following:

begin
  Process.wait(pid)
# rubocop:disable Lint/HandleExceptions
rescue Errno::ECHILD
# rubocop:enable Lint/HandleExceptions
end

disabling the cop for the whole file is undesirable, as there are many other exceptions that you would still want to enforce handling for.

Describe the solution you'd like

It would be nice to have a configuration option for the Lint/HandleExceptions cop that lets you provide a list of exceptions that are allowed to be rescued without handling. It should default to including Errno::ECHILD.

Describe alternatives you've considered

Another option would be to hard-code this case into the cop, since it's fairly common. E.g. if the begin..rescue..end block contains a Process.wait, then the cop should not complain about swallowing Errno::ECHILD

Additional context

N/A

@marcandre
Copy link
Contributor

Are you aware that the config AllowComments set to true will accept the code above (since there is a comment explaining why it's ok).

Another possibility is to have any body, e.g. nil # the child process has terminated, or :child_terminated

@marcandre
Copy link
Contributor

PS: Maybe you are using an old version of RuboCop; it's now called Lint/SuppressedException and the default for AllowComments has been set to true.

@koic
Copy link
Member

koic commented Jun 12, 2020

Lint/HandleExceptions has been renamed to Lint/SuppressedException. The latest RuboCop allows it by default. Please update RuboCop. Thank you.

@koic koic closed this as completed Jun 12, 2020
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

No branches or pull requests

3 participants