MM-11734: better plugin `error` handling (#9405)

* MM-11734: encode unregistered error implementations as an ErrorString

* MM-11734: test error string handling

* more idiomatic error handling

1 files changed, 29 insertions, 2 deletions

diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go
index d8547f352..72bd41f68 100644
--- a/plugin/client_rpc.go
+++ b/plugin/client_rpc.go
@@ -62,12 +62,39 @@ type apiRPCServer struct {
 	impl API
 }
 
+// ErrorString is a fallback for sending unregistered implementations of the error interface across
+// rpc. For example, the errorString type from the github.com/pkg/errors package cannot be
+// registered since it is not exported, but this precludes common error handling paradigms.
+// ErrorString merely preserves the string description of the error, while satisfying the error
+// interface itself to allow other registered types (such as model.AppError) to be sent unmodified.
+type ErrorString struct {
+	Err string
+}
+
+func (e ErrorString) Error() string {
+	return e.Err
+}
+
+func encodableError(err error) error {
+	if err == nil {
+		return nil
+	}
+	if _, ok := err.(*model.AppError); ok {
+		return err
+	}
+
+	return &ErrorString{
+		Err: err.Error(),
+	}
+}
+
 // Registering some types used by MM for encoding/gob used by rpc
 func init() {
 	gob.Register([]*model.SlackAttachment{})
 	gob.Register([]interface{}{})
 	gob.Register(map[string]interface{}{})
 	gob.Register(&model.AppError{})
+	gob.Register(&ErrorString{})
 }
 
 // These enforce compile time checks to make sure types implement the interface
@@ -128,7 +155,7 @@ func (s *hooksRPCServer) Implemented(args struct{}, reply *[]string) error {
 		methods = append(methods, method.Name)
 	}
 	*reply = methods
-	return nil
+	return encodableError(nil)
 }
 
 type Z_OnActivateArgs struct {
@@ -182,7 +209,7 @@ func (s *hooksRPCServer) OnActivate(args *Z_OnActivateArgs, returns *Z_OnActivat
 	if hook, ok := s.impl.(interface {
 		OnActivate() error
 	}); ok {
-		returns.A = hook.OnActivate()
+		returns.A = encodableError(hook.OnActivate())
 	}
 	return nil
 }