Gracefully handle missing node #1105
Conversation
Running this task when rails is loaded doesn't behave as expected because ActiveSupport monkeypatches Kernel#` as follows: https://github.com/rails/rails/blob/7d75599c875b081f63fc292e454b25e8e42eb60e/activesupport/lib/active_support/core_ext/kernel/agnostics.rb to suppress raising Errno::ENOENT. With this monkeypatch in place the code never reaches the fallback branch for legacy node versions introduced in rails#798 We can fix this by manually raising the exception that the activesupport monkeypatch has suppressed.
lib/tasks/webpacker/check_node.rake
Outdated
| @@ -4,6 +4,7 @@ namespace :webpacker do | |||
| begin | |||
| begin | |||
| node_version = `node -v` | |||
| raise Errno::ENOENT if node_version.blank? | |||
| rescue Errno::ENOENT | |||
| node_version = `nodejs -v` | |||
| raise Errno::ENOENT if node_version.blank? | |||
There was a problem hiding this comment.
How about if we re-raise it outside of rescue then I guess we don't need to raise twice?
There was a problem hiding this comment.
begin
node_version = `node -v`
rescue Errno::ENOENT
node_version = `nodejs -v`
end
raise Errno::ENOENT if node_version.blank?There was a problem hiding this comment.
@gauravtiwari that approach would fix the "node not installed" issue but it would still leave the change in #798 nonfunctional since the first block would never raise so the nodejs branch would never be tried.
There was a problem hiding this comment.
Part of the issue is that I'm not sure we can depend one way or the other on whether or not the activesupport monkeypatch will be in place. This approach works whether Kernel#` is monkeypatched to suppress Errno::ENOENT or not, although I admit it's not very pretty.
There was a problem hiding this comment.
This simplifies things a lot and works the same whether or not ActiveSupport has monkeypatched Kernel#`: e49d10a
Using shell syntax to choose either node or nodejs has the extra benefit of never raising Errno::ENOENT whether or not Kernel#` has been monkeypatched by ActiveSupport.
brasic
changed the title
|
Thanks @brasic 👍 💯 |
The
webpacker:check_nodetask blows up when node is not installed. The code that rescuesErrno::ENOENTdoesn't behave as expected when rails is loaded because ActiveSupport monkeypatches Kernel#` to suppress raising Errno::ENOENT. With this monkeypatch in place the code never reaches the fallback branch for legacy node versions introduced in #798.We can fix this by manually raising the exception that the activesupport monkeypatch has suppressed.
Before:
After:
Fixes #954