diff --git a/server/cmd/api/api/chromium_configure.go b/server/cmd/api/api/chromium_configure.go index d4cbab72..256954b4 100644 --- a/server/cmd/api/api/chromium_configure.go +++ b/server/cmd/api/api/chromium_configure.go @@ -695,11 +695,19 @@ func chromiumDisplayApplyWhileStopped(ctx context.Context, s *ApiService, plan * err = s.setResolutionXorgViaXrandr(ctx, w, h, rr, false) } if err != nil { - return cfg500ConfigureStep(chromiumConfigureStepDisplay, err.Error()) + s.logDisplayConfigureFailure(ctx, "chromium configure display step failed", w, h, rr, mode, err) + return chromiumDisplayConfigureFailureResponse(err) } return nil } +func chromiumDisplayConfigureFailureResponse(err error) oapi.ChromiumConfigureResponseObject { + if msg, ok := displayClientErrorMessage(err); ok { + return cfg400(fmt.Sprintf("%s: %s", chromiumConfigureStepDisplay, msg)) + } + return cfg500ConfigureStep(chromiumConfigureStepDisplay, err.Error()) +} + func chromiumRunPatchDisplay(ctx context.Context, s *ApiService, body *oapi.PatchDisplayJSONRequestBody) oapi.ChromiumConfigureResponseObject { resp, err := s.PatchDisplay(ctx, oapi.PatchDisplayRequestObject{Body: body}) if err != nil { diff --git a/server/cmd/api/api/chromium_configure_test.go b/server/cmd/api/api/chromium_configure_test.go index e4f44f04..33d31709 100644 --- a/server/cmd/api/api/chromium_configure_test.go +++ b/server/cmd/api/api/chromium_configure_test.go @@ -3,11 +3,14 @@ package api import ( "bytes" "errors" + "fmt" "io" "mime/multipart" "strings" "testing" + "github.com/kernel/kernel-images/server/lib/nekoclient" + oapi "github.com/kernel/kernel-images/server/lib/oapi" "github.com/stretchr/testify/require" ) @@ -118,6 +121,24 @@ func TestChromiumParseDisplayPartsValidation(t *testing.T) { require.Equal(t, "display payload empty", msg) } +func TestChromiumDisplayConfigureFailureResponse(t *testing.T) { + clientErr := fmt.Errorf("failed to change screen configuration: %w", &nekoclient.StatusError{ + StatusCode: 422, + Body: `{"code":422,"message":"cannot set screen size"}`, + }) + resp := chromiumDisplayConfigureFailureResponse(clientErr) + + badReq, ok := resp.(oapi.ChromiumConfigure400JSONResponse) + require.True(t, ok) + require.Contains(t, badReq.Message, "display:") + require.Contains(t, badReq.Message, "cannot set screen size") + + serverErr := errors.New("xrandr failed") + resp = chromiumDisplayConfigureFailureResponse(serverErr) + _, ok = resp.(oapi.ChromiumConfigure500JSONResponse) + require.True(t, ok) +} + func TestChromiumCfgParseMultipart(t *testing.T) { buf := bytes.NewBuffer(nil) w := multipart.NewWriter(buf) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index 955320e3..88d2ee99 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/base64" + "errors" "fmt" "log/slog" "os" @@ -14,6 +15,7 @@ import ( "github.com/kernel/kernel-images/server/lib/cdpclient" "github.com/kernel/kernel-images/server/lib/logger" + "github.com/kernel/kernel-images/server/lib/nekoclient" oapi "github.com/kernel/kernel-images/server/lib/oapi" "github.com/kernel/kernel-images/server/lib/recorder" nekooapi "github.com/m1k1o/neko/server/lib/oapi" @@ -162,7 +164,12 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ } if err != nil { - log.Error("failed to change resolution", "error", err) + s.logDisplayConfigureFailure(ctx, "failed to change resolution", width, height, refreshRate, displayMode, err) + if msg, ok := displayClientErrorMessage(err); ok { + return oapi.PatchDisplay400JSONResponse{ + BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: msg}, + }, nil + } return oapi.PatchDisplay500JSONResponse{ InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{ Message: fmt.Sprintf("failed to change resolution: %s", err.Error()), @@ -713,3 +720,48 @@ func (s *ApiService) setResolutionViaNeko(ctx context.Context, width, height, re log.Info("successfully changed resolution via Neko API", "width", width, "height", height, "refresh_rate", refreshRate) return nil } + +func displayClientErrorMessage(err error) (string, bool) { + var statusErr *nekoclient.StatusError + if errors.As(err, &statusErr) && statusErr.IsClientError() { + return err.Error(), true + } + return "", false +} + +func (s *ApiService) logDisplayConfigureFailure(ctx context.Context, msg string, width, height, refreshRate int, displayMode string, err error) { + attrs := []any{ + "error", err, + "requested_width", width, + "requested_height", height, + "requested_refresh_rate", refreshRate, + "display_mode", displayMode, + "neko_enabled", s.isNekoEnabled(), + } + attrs = append(attrs, runtimeMetadataAttrs()...) + logger.FromContext(ctx).Error(msg, attrs...) +} + +func runtimeMetadataAttrs() []any { + attrs := make([]any, 0, 16) + if hostname, err := os.Hostname(); err == nil && strings.TrimSpace(hostname) != "" { + attrs = append(attrs, "hostname", hostname) + } + for _, env := range []struct { + attr string + key string + }{ + {"instance_name", "INST_NAME"}, + {"metro", "METRO_NAME"}, + {"image", "IMAGE"}, + {"image_name", "KERNEL_IMAGE"}, + {"image_tag", "KERNEL_IMAGE_TAG"}, + {"image_sha", "KERNEL_IMAGE_SHA"}, + {"s2_stream", "S2_STREAM"}, + } { + if value := strings.TrimSpace(os.Getenv(env.key)); value != "" { + attrs = append(attrs, env.attr, value) + } + } + return attrs +} diff --git a/server/lib/nekoclient/client.go b/server/lib/nekoclient/client.go index 68fc1533..9eacc030 100644 --- a/server/lib/nekoclient/client.go +++ b/server/lib/nekoclient/client.go @@ -19,6 +19,19 @@ type AuthClient struct { password string } +type StatusError struct { + StatusCode int + Body string +} + +func (e *StatusError) Error() string { + return fmt.Sprintf("screen configuration API returned status %d: %s", e.StatusCode, e.Body) +} + +func (e *StatusError) IsClientError() bool { + return e.StatusCode >= http.StatusBadRequest && e.StatusCode < http.StatusInternalServerError +} + // NewAuthClient creates a new authenticated Neko client. func NewAuthClient(baseURL, username, password string) (*AuthClient, error) { client, err := nekooapi.NewClientWithResponses(baseURL) @@ -154,7 +167,7 @@ func (c *AuthClient) ScreenConfigurationChange(ctx context.Context, config nekoo } if resp.StatusCode() != http.StatusOK && resp.StatusCode() != http.StatusNoContent { - return fmt.Errorf("screen configuration API returned status %d: %s", resp.StatusCode(), string(resp.Body)) + return &StatusError{StatusCode: resp.StatusCode(), Body: string(resp.Body)} } return nil