mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
aa13120abf
After more investigation i've found a much better fix for #391. I'm merging this either way, because it blocks 0.13.0 release and we already have this fix tested and approved. By the next release, I want to revert this patch in favour of fixing core of the issue in simserver. Regression introduced by https://github.com/software-mansion/radon/pull/101 adding the streaming feature to the argent simulator-server build, which made `/api/screenshot` serve the last streamed frame async instead of capturing on demand. <details> ## Summary Fixes #391 — `screenshot` failing with `no image to export` when more than one simulator is booted, even with an explicit `--udid`. Regression in 0.12.x. ## Root cause The simulator-server captures screenshots from its **live frame stream**, so a capture only succeeds once the first frame has arrived. Immediately after the server starts streaming, `POST /api/screenshot` replies `HTTP 200 { error: "no image to export" }` until that first frame lands (~0.5–1s). The tool-server requested the capture right after the server became ready, **losing that race**. - With a single, foreground simulator the first frame arrives fast enough that the race is usually won — hiding the bug. - With more than one simulator booted, the target is often backgrounded and its first frame is delayed, so the race is lost **reliably** → `no image to export`. This became reachable when streaming was enabled for the argent simulator-server build in 0.12.x (radon [#101](https://github.com/software-mansion/radon/pull/101)); the 0.11.0 non-streaming build captured frames on demand, so it was immune. Device selection itself is correct — each device's stream captures the right device (verified: Pro Max → 1320×2868, iPhone 15 → 1179×2556). ## Fix `httpScreenshot` now polls past that specific transient (every 250ms, up to 6s) instead of failing immediately, while still surfacing every other error (non-2xx, Android `wrong data size`, missing url/path) right away. ## Verification End-to-end against the **shipped** simulator-server binary with **three** booted simulators, forcing a fresh sim-server spawn before each capture: | | result | |---|---| | **unfixed** (current `dist`) | `no image to export` ~2/3 of fresh-spawn attempts | | **fixed** | 7/7 success (~800ms), returns a correct PNG (1320×2868) | - `npm test` (tool-server): **1199 passed**, including new retry / give-up cases for `httpScreenshot` - `npx tsc --build`, `prettier --check`, `typecheck:tests`: clean </details>
107 lines
3.6 KiB
TypeScript
107 lines
3.6 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
|
|
import { httpScreenshot } from "../src/utils/simulator-client";
|
|
|
|
function fakeFetch(status: number, json: unknown) {
|
|
return vi.fn(
|
|
async () =>
|
|
({
|
|
ok: status >= 200 && status < 300,
|
|
status,
|
|
json: async () => json,
|
|
}) as unknown as Response
|
|
);
|
|
}
|
|
|
|
const api = { apiUrl: "http://127.0.0.1:4949" } as never;
|
|
|
|
describe("httpScreenshot", () => {
|
|
afterEach(() => {
|
|
vi.unstubAllGlobals();
|
|
});
|
|
|
|
it("surfaces the server's error message when a 200 response carries { error } and no url/path", async () => {
|
|
// The Android simulator-server reports a full-resolution framebuffer
|
|
// mismatch as HTTP 200 + { error }, not a non-2xx status. The real cause
|
|
// must reach the caller, not the misleading "restart the server" hint.
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
fakeFetch(200, { error: "wrong data size, expected 7853760 got 17627328" })
|
|
);
|
|
await expect(httpScreenshot(api)).rejects.toThrow(
|
|
"Screenshot failed: wrong data size, expected 7853760 got 17627328."
|
|
);
|
|
});
|
|
|
|
it("uses the generic hint only when url/path are missing AND there is no error field", async () => {
|
|
vi.stubGlobal("fetch", fakeFetch(200, {}));
|
|
await expect(httpScreenshot(api)).rejects.toThrow("server response missing url or path");
|
|
});
|
|
|
|
it("returns url and path on a successful capture", async () => {
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
fakeFetch(200, { url: "http://127.0.0.1:4949/media/x.png", path: "/tmp/x.png" })
|
|
);
|
|
await expect(httpScreenshot(api)).resolves.toEqual({
|
|
url: "http://127.0.0.1:4949/media/x.png",
|
|
path: "/tmp/x.png",
|
|
});
|
|
});
|
|
|
|
it("retries 'no image to export' until the first frame lands, then resolves", async () => {
|
|
// A freshly-spawned simulator-server has not captured its first frame yet,
|
|
// so the streaming screenshot endpoint replies 200 { error: "no image to
|
|
// export" } for ~0.5-1s. The capture must poll past that rather than fail
|
|
// (regression #391: reliably hit with >1 booted simulator).
|
|
vi.useFakeTimers();
|
|
try {
|
|
let calls = 0;
|
|
vi.stubGlobal(
|
|
"fetch",
|
|
vi.fn(async () => {
|
|
calls += 1;
|
|
const json =
|
|
calls < 3
|
|
? { error: "no image to export" }
|
|
: { url: "http://127.0.0.1:4949/media/x.png", path: "/tmp/x.png" };
|
|
return { ok: true, status: 200, json: async () => json } as unknown as Response;
|
|
})
|
|
);
|
|
const pending = httpScreenshot(api);
|
|
await vi.advanceTimersByTimeAsync(600);
|
|
await expect(pending).resolves.toEqual({
|
|
url: "http://127.0.0.1:4949/media/x.png",
|
|
path: "/tmp/x.png",
|
|
});
|
|
expect(calls).toBe(3);
|
|
} finally {
|
|
vi.useRealTimers();
|
|
}
|
|
});
|
|
|
|
it("gives up with the 'no image to export' message once the first-frame deadline passes", async () => {
|
|
vi.useFakeTimers();
|
|
try {
|
|
const fetchMock = vi.fn(
|
|
async () =>
|
|
({
|
|
ok: true,
|
|
status: 200,
|
|
json: async () => ({ error: "no image to export" }),
|
|
}) as unknown as Response
|
|
);
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
const pending = httpScreenshot(api);
|
|
const expectation = expect(pending).rejects.toThrow("Screenshot failed: no image to export.");
|
|
await vi.advanceTimersByTimeAsync(7_000);
|
|
await expectation;
|
|
// Polling is bounded (~6s / 250ms ≈ 24 attempts), never an infinite loop.
|
|
expect(fetchMock.mock.calls.length).toBeGreaterThan(1);
|
|
expect(fetchMock.mock.calls.length).toBeLessThan(40);
|
|
} finally {
|
|
vi.useRealTimers();
|
|
}
|
|
});
|
|
});
|