Return 400 when Neko rejects a display configuration

server/cmd/api/api/chromium_configure.go

@@ -17,6 +17,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/policy"
 	"github.com/kernel/kernel-images/server/lib/zstdutil"
@@ -842,6 +843,9 @@ func chromiumDisplayApplyWhileStopped(ctx context.Context, s *ApiService, plan *
 		return nil
 	}
 	_, _, err := s.applyResolutionAndConverge(ctx, w, h, rr, s.isNekoEnabled())
+	if errors.Is(err, nekoclient.ErrScreenConfigurationRejected) {
+		return cfg400(unsupportedDisplayMessage(w, h, rr))
+	}
 	if err != nil {
 		return cfg500ConfigureStep(chromiumConfigureStepDisplay, err.Error())
 	}

server/cmd/api/api/display.go

@@ -15,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"
@@ -184,6 +185,9 @@ func (s *ApiService) patchDisplayLocked(ctx context.Context, req oapi.PatchDispl
 
 	if err != nil {
 		log.Error("failed to change resolution", "error", err)
+		if errors.Is(err, nekoclient.ErrScreenConfigurationRejected) {
+			return oapi.PatchDisplay400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: unsupportedDisplayMessage(width, height, refreshRate)}}, nil
+		}
 		return oapi.PatchDisplay500JSONResponse{
 			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{
 				Message: fmt.Sprintf("failed to change resolution: %s", err.Error()),
@@ -946,3 +950,9 @@ 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
 }
+
+// unsupportedDisplayMessage is the client error for a display configuration
+// the X server refused to set.
+func unsupportedDisplayMessage(width, height, refreshRate int) string {
+	return fmt.Sprintf("the display cannot be set to %dx%d at %d Hz", width, height, refreshRate)
+}

server/cmd/api/api/display_test.go

@@ -6,13 +6,16 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"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"
 	"github.com/kernel/kernel-images/server/lib/scaletozero"
@@ -611,3 +614,79 @@ func TestChromiumRunPatchDisplayDoesNotRelockChromiumConfig(t *testing.T) {
 		t.Fatal("chromiumRunPatchDisplay tried to reacquire chromiumConfigMu")
 	}
 }
+
+// newRejectingNekoDisplay points the service at a headful display whose Neko
+// answers every screen change with screenStatus, and whose xrandr reports
+// 1920x1080 at 25 Hz.
+func newRejectingNekoDisplay(t *testing.T, screenStatus int) *ApiService {
+	t.Helper()
+
+	neko := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.URL.Path {
+		case "/api/login":
+			_, _ = w.Write([]byte(`{"token":"t"}`))
+		case "/api/room/screen":
+			w.WriteHeader(screenStatus)
+			_, _ = w.Write([]byte(`{"code":422,"message":"cannot set screen size"}`))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	t.Cleanup(neko.Close)
+
+	binDir := t.TempDir()
+	xrandr := "#!/bin/sh\necho '   1920x1080_25.00  25.00*'\n"
+	require.NoError(t, os.WriteFile(filepath.Join(binDir, "xrandr"), []byte(xrandr), 0o755))
+	t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH"))
+	t.Setenv("ENABLE_WEBRTC", "true")
+
+	origConf := xvfbSupervisorConf
+	xvfbSupervisorConf = filepath.Join(t.TempDir(), "absent.conf")
+	t.Cleanup(func() { xvfbSupervisorConf = origConf })
+
+	client, err := nekoclient.NewAuthClient(neko.URL, "admin", "admin")
+	require.NoError(t, err)
+	svc := newTestServiceWithFactory(t, newMockRecordManager(), newMockFactory())
+	svc.nekoAuthClient = client
+	return svc
+}
+
+func TestPatchDisplayNekoRejection(t *testing.T) {
+	width, height := 1008, 384
+	rate := oapi.PatchDisplayRequestRefreshRate(60)
+	requireIdle := false
+	req := oapi.PatchDisplayRequestObject{Body: &oapi.PatchDisplayJSONRequestBody{Width: &width, Height: &height, RefreshRate: &rate, RequireIdle: &requireIdle}}
+
+	t.Run("422 is a client error", func(t *testing.T) {
+		svc := newRejectingNekoDisplay(t, http.StatusUnprocessableEntity)
+		resp, err := svc.PatchDisplay(context.Background(), req)
+		require.NoError(t, err)
+		require.IsType(t, oapi.PatchDisplay400JSONResponse{}, resp)
+		assert.Equal(t, "the display cannot be set to 1008x384 at 60 Hz", resp.(oapi.PatchDisplay400JSONResponse).Message)
+	})
+
+	t.Run("500 stays a server error", func(t *testing.T) {
+		svc := newRejectingNekoDisplay(t, http.StatusInternalServerError)
+		resp, err := svc.PatchDisplay(context.Background(), req)
+		require.NoError(t, err)
+		require.IsType(t, oapi.PatchDisplay500JSONResponse{}, resp)
+	})
+}
+
+func TestChromiumDisplayApplyWhileStoppedNekoRejection(t *testing.T) {
+	plan := &chromiumDisplayPlan{width: 1008, height: 384, refreshRate: 60}
+
+	t.Run("422 is a client error", func(t *testing.T) {
+		svc := newRejectingNekoDisplay(t, http.StatusUnprocessableEntity)
+		resp := chromiumDisplayApplyWhileStopped(context.Background(), svc, plan)
+		require.IsType(t, oapi.ChromiumConfigure400JSONResponse{}, resp)
+		assert.Equal(t, "the display cannot be set to 1008x384 at 60 Hz", resp.(oapi.ChromiumConfigure400JSONResponse).Message)
+	})
+
+	t.Run("500 stays a server error", func(t *testing.T) {
+		svc := newRejectingNekoDisplay(t, http.StatusInternalServerError)
+		resp := chromiumDisplayApplyWhileStopped(context.Background(), svc, plan)
+		require.IsType(t, oapi.ChromiumConfigure500JSONResponse{}, resp)
+	})
+}

server/lib/nekoclient/client.go

@@ -2,13 +2,19 @@ package nekoclient
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"sync"
 
 	nekooapi "github.com/m1k1o/neko/server/lib/oapi"
 )
 
+// ErrScreenConfigurationRejected is returned when Neko answers a screen
+// configuration change with 422, which it does when the X server refuses the
+// requested size or refresh rate. Retrying the same request cannot succeed.
+var ErrScreenConfigurationRejected = errors.New("screen configuration rejected")
+
 // AuthClient wraps the Neko OpenAPI client and handles authentication automatically.
 // It manages token caching and refresh on 401 responses.
 type AuthClient struct {
@@ -157,6 +163,9 @@ func (c *AuthClient) ScreenConfigurationChange(ctx context.Context, config nekoo
 		}
 	}
 
+	if resp.StatusCode() == http.StatusUnprocessableEntity {
+		return fmt.Errorf("%w: screen configuration API returned status %d: %s", ErrScreenConfigurationRejected, resp.StatusCode(), string(resp.Body))
+	}
 	if resp.StatusCode() != http.StatusOK && resp.StatusCode() != http.StatusNoContent {
 		return fmt.Errorf("screen configuration API returned status %d: %s", resp.StatusCode(), string(resp.Body))
 	}

server/lib/nekoclient/client_test.go

@@ -0,0 +1,56 @@
+package nekoclient
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	nekooapi "github.com/m1k1o/neko/server/lib/oapi"
+)
+
+func TestScreenConfigurationChangeStatus(t *testing.T) {
+	cases := []struct {
+		name         string
+		status       int
+		wantErr      bool
+		wantRejected bool
+	}{
+		{name: "ok", status: http.StatusOK},
+		{name: "unprocessable", status: http.StatusUnprocessableEntity, wantErr: true, wantRejected: true},
+		{name: "server error", status: http.StatusInternalServerError, wantErr: true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				switch r.URL.Path {
+				case "/api/login":
+					_, _ = w.Write([]byte(`{"token":"t"}`))
+				case "/api/room/screen":
+					w.WriteHeader(tc.status)
+					_, _ = w.Write([]byte(`{}`))
+				default:
+					http.NotFound(w, r)
+				}
+			}))
+			defer srv.Close()
+
+			client, err := NewAuthClient(srv.URL, "admin", "admin")
+			if err != nil {
+				t.Fatal(err)
+			}
+			width, height, rate := 1008, 384, 60
+			err = client.ScreenConfigurationChange(context.Background(), nekooapi.ScreenConfiguration{Width: &width, Height: &height, Rate: &rate})
+
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("got error %v, want error %v", err, tc.wantErr)
+			}
+			if got := errors.Is(err, ErrScreenConfigurationRejected); got != tc.wantRejected {
+				t.Fatalf("errors.Is(err, ErrScreenConfigurationRejected) = %v, want %v (err: %v)", got, tc.wantRejected, err)
+			}
+		})
+	}
+}