Loading invalid plugin gives non-useful error

[lynxbat]

When loading a plugin that should not run the error does not fit:

Error loading plugin:
timed out waiting for plugin snap-plugin-publisher-influxdb-built

In this case, the influxdb plugin is a Linux build but I am loading on my Mac. I believe this used to return the error from exec on the plugin.

[geauxvirtual]

PR #1104 changed this behavior.

[IRCody]

If we timeout we may or may not have any output from the plugin. A trivial example:

% echo "sleep 60"  > test
% chmod a+x test
% snapctl plugin load test
Error loading plugin:
timed out waiting for plugin test

So I think we need to have the timed out message (or something equivalent). It makes sense to add in any output from the plugin to the tail of the error message. This would give the user more context (when available) about what went wrong.

PR #1104 changed this behavior.

#1104 isn't a PR. I don't see how the pr linked to that issue ( #1106) changed this behavior. Can you elaborate?

[geauxvirtual]

You are correct, PR #1106 (#1104 was the issue).

Trying this same behavior in release v0.15.0, the output will actually be a runtime error PANIC for invalid memory address or nil pointer deference when trying to load a plugin that can not execute on the system.

After PR #1106, a warning is seen in the snapd output saying Process for plugin 'snap-plugin-collector-mock1' not started; cannot kill _block=Kill, which was added as part of #1106. I would say the timeout message is at least better than the PANIC seen in v0.15.0 for this same.

[geauxvirtual]

The actual error from start is never being captured, as seen here: https://github.com/intelsdi-x/snap/blob/master/control/plugin/execution.go#L76

If this error is actually captured, trying to start a plugin that doesn't match the underlying architecture of the system would result in this type of error:

--- intelsdi-x/snap ‹master* ?› » go run cmd/snapctl/*.go plugin load ~/Downloads/snap-v0.16.1-beta/plugin/snap-plugin-collector-mock1                                           1 ↵
Error loading plugin:
fork/exec /var/folders/q7/4hp8_tnx02n2wd4py2rnd4dh0000gq/T/715241071/snap-plugin-collector-mock1: exec format error

[geauxvirtual]

Executable start errors were captured prior to #1055.

[IRCody]

Thanks for digging deeper into it. I wonder if you could capture it fairly easily on the command wrapper struct and add something to the command interface (Maybe GetLastError() error) to allow it to be retrieved by the executablePlugin. Does that seem like a reasonable approach?

[geauxvirtual]

In testing this, the only changes that were need were to update the command interface to

type command interface {
    Start() error

And the function: func (cw *commandWrapper) Start() error { return cw.cmd.Start() }

From there, inside of Run just needs to check to see if Start() returned an error, and return that error. The error could be better formatted to not include the temporary path and be more pertinent to what the user was trying to load.