diff --git a/pkg/client/errors_test.go b/pkg/client/errors_test.go index 78ad640..cde3895 100644 --- a/pkg/client/errors_test.go +++ b/pkg/client/errors_test.go @@ -59,8 +59,9 @@ func TestClient_Post_ErrorsResponse(t *testing.T) { t.Errorf("expected message '%s', got '%s'", expectedMsg, errs.Errors[0].Message) } - if err.Error() != expectedMsg { - t.Errorf("expected Error() to return '%s', got '%s'", expectedMsg, err.Error()) + expectedErr := "UNKNOWN_ACTION_ERROR: " + expectedMsg + if err.Error() != expectedErr { + t.Errorf("expected Error() to return '%s', got '%s'", expectedErr, err.Error()) } } diff --git a/pkg/models/device.go b/pkg/models/device.go index bd55a0f..6c08580 100644 --- a/pkg/models/device.go +++ b/pkg/models/device.go @@ -2,6 +2,8 @@ package models import ( "encoding/xml" + "fmt" + "strconv" "time" ) @@ -80,7 +82,7 @@ type ErrorsResponse struct { // Error implements the error interface for ErrorsResponse func (e *ErrorsResponse) Error() string { if len(e.Errors) > 0 { - return e.Errors[0].Message + return e.Errors[0].Error() } return "unknown API error" @@ -93,6 +95,27 @@ type DeviceError struct { Message string `xml:",chardata"` } +// Error implements the error interface for DeviceError. Some speakers +// return a Message that just restates Value as text (e.g. a bare "1047" +// for an error the firmware has no localized string for) — Name is the +// only informative part in that case, so it's always included unless +// Message already carries it. +func (e DeviceError) Error() string { + if e.Name == "" { + if e.Message == "" { + return fmt.Sprintf("device error %d", e.Value) + } + + return e.Message + } + + if e.Message == "" || e.Message == e.Name || e.Message == strconv.Itoa(e.Value) { + return fmt.Sprintf("%s (%d)", e.Name, e.Value) + } + + return fmt.Sprintf("%s: %s", e.Name, e.Message) +} + // DiscoveredDevice represents a device found through network discovery type DiscoveredDevice struct { Name string `json:"name"` diff --git a/pkg/models/device_error_test.go b/pkg/models/device_error_test.go new file mode 100644 index 0000000..c89df0c --- /dev/null +++ b/pkg/models/device_error_test.go @@ -0,0 +1,69 @@ +package models + +import "testing" + +func TestDeviceError_Error(t *testing.T) { + tests := []struct { + name string + err DeviceError + expected string + }{ + { + name: "message repeats the numeric value (real speaker case)", + err: DeviceError{Value: 1047, Name: "SOURCE_ALREADY_REMOVED", Message: "1047"}, + expected: "SOURCE_ALREADY_REMOVED (1047)", + }, + { + name: "message is empty", + err: DeviceError{Value: 1047, Name: "SOURCE_ALREADY_REMOVED", Message: ""}, + expected: "SOURCE_ALREADY_REMOVED (1047)", + }, + { + name: "message is meaningful and distinct from name", + err: DeviceError{Value: 1029, Name: "UNKNOWN_ACTION_ERROR", Message: "This version of SCM does not support spotify create account functionality."}, + expected: "UNKNOWN_ACTION_ERROR: This version of SCM does not support spotify create account functionality.", + }, + { + name: "name is empty, message carries the detail", + err: DeviceError{Value: 500, Name: "", Message: "internal error"}, + expected: "internal error", + }, + { + name: "both name and message are empty", + err: DeviceError{Value: 500, Name: "", Message: ""}, + expected: "device error 500", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.err.Error(); got != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, got) + } + }) + } +} + +func TestErrorsResponse_Error(t *testing.T) { + t.Run("delegates to the first DeviceError", func(t *testing.T) { + errs := &ErrorsResponse{ + Errors: []DeviceError{ + {Value: 1047, Name: "SOURCE_ALREADY_REMOVED", Message: "1047"}, + }, + } + + expected := "SOURCE_ALREADY_REMOVED (1047)" + if got := errs.Error(); got != expected { + t.Errorf("expected %q, got %q", expected, got) + } + }) + + t.Run("no errors", func(t *testing.T) { + errs := &ErrorsResponse{} + + expected := "unknown API error" + if got := errs.Error(); got != expected { + t.Errorf("expected %q, got %q", expected, got) + } + }) +}