cli-plugins/manager: fix Plugin marshaling with regular errors

Go does not by default marshal `error` type fields to JSON. The manager
package therefore implemented a `pluginError` type that implements
[encoding.TextMarshaler]. However, the field was marked as a regular
`error`, which made it brittle; assining any other type of error would
result in the error being discarded in the marshaled JSON (as used in
`docker info` output), resulting in the error being marshaled as `{}`.

This patch adds a custom `MarshalJSON()` on the `Plugin` type itself
so that any error is rendered. It checks if the error used already
implements [encoding.TextMarshaler], otherwise wraps the error in
a `pluginError`.

[encoding.TextMarshaler]: https://pkg.go.dev/encoding#TextMarshaler

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

cli-plugins/manager/plugin.go

@@ -2,6 +2,7 @@ package manager
 
 import (
 	"context"
+	"encoding"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -31,6 +32,22 @@ type Plugin struct {
 	ShadowedPaths []string `json:",omitempty"`
 }
 
+// MarshalJSON implements [json.Marshaler] to handle marshaling the
+// [Plugin.Err] field (Go doesn't marshal errors by default).
+func (p *Plugin) MarshalJSON() ([]byte, error) {
+	type Alias Plugin // avoid recursion
+
+	cp := *p // shallow copy to avoid mutating original
+
+	if cp.Err != nil {
+		if _, ok := cp.Err.(encoding.TextMarshaler); !ok {
+			cp.Err = &pluginError{cp.Err}
+		}
+	}
+
+	return json.Marshal((*Alias)(&cp))
+}
+
 // pluginCandidate represents a possible plugin candidate, for mocking purposes.
 type pluginCandidate interface {
 	Path() string

cli-plugins/manager/plugin_test.go

@@ -0,0 +1,43 @@
+package manager
+
+import (
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestPluginMarshal(t *testing.T) {
+	const jsonWithError = `{"Name":"some-plugin","Err":"something went wrong"}`
+	const jsonNoError = `{"Name":"some-plugin"}`
+
+	tests := []struct {
+		doc      string
+		error    error
+		expected string
+	}{
+		{
+			doc:      "no error",
+			expected: jsonNoError,
+		},
+		{
+			doc:      "regular error",
+			error:    errors.New("something went wrong"),
+			expected: jsonWithError,
+		},
+		{
+			doc:      "custom error",
+			error:    NewPluginError("something went wrong"),
+			expected: jsonWithError,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			actual, err := json.Marshal(&Plugin{Name: "some-plugin", Err: tc.error})
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(string(actual), tc.expected))
+		})
+	}
+}

cli/command/system/info_test.go

@@ -2,6 +2,7 @@ package system
 
 import (
 	"encoding/base64"
+	"errors"
 	"net"
 	"testing"
 	"time"
@@ -226,7 +227,7 @@ var samplePluginsInfo = []pluginmanager.Plugin{
 	{
 		Name: "badplugin",
 		Path: "/path/to/docker-badplugin",
-		Err:  pluginmanager.NewPluginError("something wrong"),
+		Err:  errors.New("something wrong"),
 	},
 }