From 549d39a89fe3e6546c55af2a00c9ea0b77305ef3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 4 Aug 2025 10:45:02 +0200 Subject: [PATCH] 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 --- cli-plugins/manager/plugin.go | 17 ++++++++++++ cli-plugins/manager/plugin_test.go | 43 ++++++++++++++++++++++++++++++ cli/command/system/info_test.go | 3 ++- 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 cli-plugins/manager/plugin_test.go diff --git a/cli-plugins/manager/plugin.go b/cli-plugins/manager/plugin.go index ed4dac54e..e72d9039b 100644 --- a/cli-plugins/manager/plugin.go +++ b/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 diff --git a/cli-plugins/manager/plugin_test.go b/cli-plugins/manager/plugin_test.go new file mode 100644 index 000000000..84d71d518 --- /dev/null +++ b/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)) + }) + } +} diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index 1591ed833..adf021d49 100644 --- a/cli/command/system/info_test.go +++ b/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"), }, }