fix: the API emitted undefined optionals as null, and the UI couldn't decode them (v0.3.2)
`Schema.optional(X)` means `X | undefined`, and the plain Schema encoder omits a
present-but-undefined key — but the HttpApi RESPONSE path serialises it as
`null`, which `X | undefined` then refuses when the client decodes it. Two
user-visible failures shipped in 0.3.1:
- GET /api/status on a never-synced host returned `"lastSync":null`, so the
Overview page could not decode its own status — i.e. every fresh install
until the first successful sync.
- POST /api/detect on a box with any emulator installed returned
`"contested":null` / `"appId":null`, so the Emulators page's Detect button
could not decode the result. It only looked fine because a value that has
round-tripped through cache.json has those keys ABSENT rather than present
-and-undefined, and absent keys encode correctly.
Nothing on this wire is `undefined` any more. EngineStatus.{lastSync,lastReport,
detectedAt} are `Schema.NullOr` and the engine sends explicit nulls. The
DetectedEmulator fields use `Schema.optional(Schema.NullOr(...))` instead —
tolerant of absent, undefined and null — because they also live in cache.json,
and requiring them would fail the whole cache decode on upgrade and silently
discard every art verdict.
plugin/test/wire.test.ts pins it: it drives the real makeApi handler stack
through toWebHandler and decodes the bytes with the shared contract schemas.
Against the old schema all five of its assertions fail; a unit test of either
side in isolation passes while the product is broken.
Also: the SSE feed now emits the current status on subscribe rather than only on
the next engine transition, so a freshly-opened console page is correct
immediately instead of stale until something happens.
This commit is contained in:
@@ -0,0 +1,178 @@
|
||||
// The wire contract: whatever the server ENCODES, the client must be able to DECODE.
|
||||
//
|
||||
// This exists because a whole class of bug is invisible to unit tests of either side.
|
||||
// `Schema.optional(X)` describes `X | undefined`, and the plain Schema encoder omits a
|
||||
// present-but-undefined key — but the HttpApi RESPONSE path serialises it as `null`, which
|
||||
// `X | undefined` then refuses on the way back in. The result shipped in 0.3.1: a
|
||||
// never-synced host's `GET /api/status` returned `"lastSync":null` and the Overview page
|
||||
// could not decode it, and `POST /api/detect` on a box with any emulator installed returned
|
||||
// `"contested":null` and the Emulators page could not decode that.
|
||||
//
|
||||
// So these tests drive the REAL `makeApi` handler stack through `toWebHandler` and decode
|
||||
// the bytes with the shared contract schemas. A unit test of the engine, or of the schema
|
||||
// in isolation, passes happily while the product is broken.
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import type {
|
||||
CacheStore,
|
||||
ConfigService,
|
||||
SyncEngine,
|
||||
} from "@punktfunk/plugin-kit";
|
||||
import {
|
||||
type DetectedEmulator,
|
||||
EmulatorsPayload,
|
||||
EngineStatus,
|
||||
type PreviewResult,
|
||||
type RomConfigSchema,
|
||||
resolveConfig,
|
||||
type SyncReport,
|
||||
} from "@rom-manager/contract";
|
||||
import { Effect, Schema, Stream } from "effect";
|
||||
import { HttpRouter } from "effect/unstable/http";
|
||||
import { makeApi } from "../src/services/api.js";
|
||||
import type { CacheSchema } from "../src/services/cache.js";
|
||||
import type { RomSyncService } from "../src/services/engine.js";
|
||||
|
||||
const EMPTY_REPORT: SyncReport = {
|
||||
considered: 0,
|
||||
included: 0,
|
||||
skipped: [],
|
||||
excluded: [],
|
||||
warnings: [],
|
||||
truncated: 0,
|
||||
perPlatform: {},
|
||||
overWarn: false,
|
||||
};
|
||||
|
||||
/** A never-synced, never-detected host — what every fresh install looks like. */
|
||||
const FIRST_RUN_STATUS: EngineStatus = {
|
||||
rootsConfigured: 0,
|
||||
os: "linux",
|
||||
artProvider: null,
|
||||
syncing: false,
|
||||
lastSync: null,
|
||||
lastReport: null,
|
||||
detectedAt: null,
|
||||
paths: { dir: "/s", config: "/s/config.json", cache: "/s/cache.json" },
|
||||
};
|
||||
|
||||
/**
|
||||
* A detection result exactly as `detectAll` builds it: the optional keys are PRESENT with
|
||||
* `undefined` values, not absent. That distinction is the whole bug — a value round-tripped
|
||||
* through cache.json has them absent instead, and absent keys encode correctly, which is
|
||||
* why this only ever broke right after a live detect.
|
||||
*/
|
||||
const FRESHLY_DETECTED: DetectedEmulator = {
|
||||
id: "duckstation",
|
||||
name: "DuckStation",
|
||||
template: "{exe} -batch {rom}",
|
||||
supportsArchives: true,
|
||||
contested: undefined,
|
||||
exeToken: "duckstation-qt",
|
||||
exePath: "/usr/bin/duckstation-qt",
|
||||
appId: undefined,
|
||||
via: "path",
|
||||
coresDir: undefined,
|
||||
cores: undefined,
|
||||
};
|
||||
|
||||
const stubDeps = () => {
|
||||
const engine = {
|
||||
sync: () => Effect.succeed({ _tag: "Unchanged", report: EMPTY_REPORT }),
|
||||
status: Effect.succeed({ syncing: false }),
|
||||
changes: Stream.empty,
|
||||
start: Effect.void,
|
||||
reconfigure: Effect.void,
|
||||
} as unknown as SyncEngine<SyncReport>;
|
||||
|
||||
const sync: RomSyncService = {
|
||||
engine,
|
||||
preview: Effect.succeed({
|
||||
entries: [],
|
||||
report: EMPTY_REPORT,
|
||||
} satisfies PreviewResult),
|
||||
detect: () => Effect.succeed([FRESHLY_DETECTED]),
|
||||
status: Effect.succeed(FIRST_RUN_STATUS),
|
||||
statusChanges: Stream.empty,
|
||||
};
|
||||
|
||||
const config = {
|
||||
load: Effect.succeed(resolveConfig({})),
|
||||
loadRaw: Effect.succeed({}),
|
||||
saveRaw: () => Effect.succeed(resolveConfig({})),
|
||||
changes: Stream.empty,
|
||||
path: "/s/config.json",
|
||||
} as unknown as ConfigService<typeof RomConfigSchema>;
|
||||
|
||||
const cache = {
|
||||
get: Effect.succeed({ art: {} }),
|
||||
modify: () => Effect.succeed(undefined),
|
||||
update: () => Effect.void,
|
||||
path: "/s/cache.json",
|
||||
} as unknown as CacheStore<typeof CacheSchema>;
|
||||
|
||||
return { sync, config, cache };
|
||||
};
|
||||
|
||||
/** Drive one request through the real handler stack and hand back the parsed body. */
|
||||
const call = async (method: string, path: string): Promise<unknown> => {
|
||||
const { handler, dispose } = HttpRouter.toWebHandler(makeApi(stubDeps()));
|
||||
try {
|
||||
const response = await handler(
|
||||
new Request(`http://localhost${path}`, { method }),
|
||||
);
|
||||
expect(response.status).toBe(200);
|
||||
return await response.json();
|
||||
} finally {
|
||||
await dispose();
|
||||
}
|
||||
};
|
||||
|
||||
describe("the status endpoint survives a round trip", () => {
|
||||
test("a never-synced host's status decodes on the client", async () => {
|
||||
const body = await call("GET", "/api/status");
|
||||
// The assertion that matters: the UI's own decoder accepts the server's bytes.
|
||||
expect(() => Schema.decodeUnknownSync(EngineStatus)(body)).not.toThrow();
|
||||
});
|
||||
|
||||
test("absent values arrive as null, not as a missing key", async () => {
|
||||
// Pinning the shape, not just its decodability: the UI branches on `=== null`.
|
||||
const body = (await call("GET", "/api/status")) as Record<string, unknown>;
|
||||
expect(body.lastSync).toBeNull();
|
||||
expect(body.lastReport).toBeNull();
|
||||
expect(body.detectedAt).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("the emulator endpoints survive a round trip", () => {
|
||||
test("a freshly detected emulator decodes on the client", async () => {
|
||||
const body = await call("POST", "/api/detect");
|
||||
expect(() =>
|
||||
Schema.decodeUnknownSync(EmulatorsPayload)(body),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("the cached-read path decodes too", async () => {
|
||||
const body = await call("GET", "/api/emulators");
|
||||
expect(() =>
|
||||
Schema.decodeUnknownSync(EmulatorsPayload)(body),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("the trap itself", () => {
|
||||
test("the HttpApi response path turns undefined into null", async () => {
|
||||
// Documents WHY the contract avoids `Schema.optional(X)` on this API. If a future
|
||||
// effect release stops doing this, the schemas can relax — this test will say so.
|
||||
const body = (await call("POST", "/api/detect")) as {
|
||||
detected: Array<Record<string, unknown>>;
|
||||
};
|
||||
const emulator = body.detected[0]!;
|
||||
expect(emulator.contested).toBeNull();
|
||||
expect(emulator.appId).toBeNull();
|
||||
// …while the plain Schema encoder omits the very same keys.
|
||||
const encoded = Schema.encodeUnknownSync(EngineStatus)({
|
||||
...FIRST_RUN_STATUS,
|
||||
}) as Record<string, unknown>;
|
||||
expect(encoded).toHaveProperty("lastSync");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user