reject nil as a command input
#1863
Conversation
arlimus
force-pushed
the
dr/cmd-nil-fix
branch
3 times, most recently
from
b8e63c0
to
5a425a1
Compare
lib/resources/command.rb
Outdated
| @@ -28,6 +28,9 @@ def initialize(cmd) | |||
| end | |||
|
|
|||
| def result | |||
| if @command.nil? | |||
| raise 'InSpec `command` was called with `nil` as the argument. This is not supported. Please provide a valid command instead.' | |||
There was a problem hiding this comment.
This feels like it belongs in #initialize since no method in this resource should work / will work with a nil command. Is there a technical reason you put it here instead of in the constructor?
There was a problem hiding this comment.
Lets make it skip the command resource for now until we have proper exception handling
There was a problem hiding this comment.
Actually, let's not skip. Skip points to unsupported use on a platform, at least in the current context. This however is a full user-error and should never be done this way. It's like adding nil to a number ;) We need the exception handling piece but it needs to fail afaics.
I can move it to initialize as well, will update this pr...
There was a problem hiding this comment.
@arlimus this will stop inspec profile execution, even if you have multiple profiles in execution
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
There was a problem hiding this comment.
Even though we try to avoid calling raise because it will unwind the whole run, I feel like an issue like this is worthy of it as it points to an implementation issue vs. an error on the target. This will obviously get better once we implement better exception handling.
It is not valid and the errors get very cryptic once it passes train and goes to the various backends. Let's just catch it early on. It is a user error that should be avoided.