mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
d4b9818d64
Fixes two confirmed defects in the JS-runtime debugger subsystem.
## Defect A — `debugger-component-tree` hangs ~15s and throws on every
error condition
**File:**
`packages/tool-server/src/utils/debugger/scripts/component-tree.ts`
The injected fiber-walk script delivers its success result through the
`__argent_callback` CDP binding, but every error path instead did
`return JSON.stringify({ error: ... })` from the async IIFE — `'No
DevTools hook'`, `'No fiber roots'`, `'Could not find UIManager'`.
The consumer (`evaluateWithBinding`, `cdp-client.ts`) runs
`Runtime.evaluate` with `returnByValue:false`/`awaitPromise:false`, so
the IIFE's return value is discarded. The promise only settles when the
binding fires or the 15s timeout elapses. So on a release/production
build, before the first render, or on a non-RN runtime,
`debugger-component-tree` blocked for ~15s and then threw a generic
`DEBUGGER_CDP_BINDING_TIMEOUT` instead of returning the intended
diagnostic immediately.
**Observed:** callback never invoked (0 calls) on the error paths; the
tool hangs to the binding timeout.
**Expected:** the error is delivered through the binding right away and
surfaces as `Error: <message>` via the consumer's
`JSON.parse(response.result).error` path.
**Fix:** mirror the sibling `inspect-at-point.ts`. Added an
`__argent_fail(msg)` helper that routes the error through
`__argent_callback` in the exact same wire shape as the success path (`{
requestId, result: JSON.stringify({ error: msg }) }`), and converted all
three error paths to call it and `return`.
## Defect B — `LogFileWriter` truncates console levels longer than 5
chars
**File:** `packages/tool-server/src/utils/debugger/log-file-writer.ts`
The flat-line level was rendered as `LEVEL_DISPLAY[entry.level] ??
entry.level.toUpperCase().padEnd(5).slice(0, 5)`. The `.slice(0,5)`
truncated any level not in the 5-entry `LEVEL_DISPLAY` map. CDP's
`Runtime.consoleAPICalled.type` includes `"warning"` (`console.warn`)
and `"assert"` (`console.assert`) — neither is in the map — so
`readAll()`/`readFiltered()` returned `"warni"`/`"asser"`, and
`readFiltered({ level: "warning" })` matched nothing.
**Observed:** writing level `"warning"` → `readAll()[0].level ===
"warni"`; `readFiltered({ level: "assert" }).total === 0`.
**Expected:** the canonical level round-trips exactly for levels of any
length.
**Fix:** drop `.slice(0,5)` — keep `padEnd(5)` for column alignment but
never cut. `LINE_RE` captures the level as `\S+`, so a non-truncated,
whitespace-free level round-trips through `parseFlatLine` unchanged.
Short levels (log/info/warn/error/debug) are unaffected.
## Tests
- `packages/tool-server/test/debugger/comptree-error-channel.test.ts` —
asserts `__argent_callback` is invoked on the error paths and the error
is encoded as `parsed.error`.
- `packages/tool-server/test/debugger/log-file-writer-level.test.ts` —
asserts `"warning"`/`"assert"` round-trip and filter correctly, and
short levels still work.
Both new files fail before the fix and pass after. Full `tool-server`
suite green (1627 tests); repo-root `tsc --build` clean; eslint clean on
changed files.
59 lines
2.9 KiB
TypeScript
59 lines
2.9 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
|
import { consoleTimestampToIso } from "../../src/utils/debugger/console-timestamp";
|
|
|
|
/**
|
|
* Regression for the RN log timestamp corruption: CDP `consoleAPICalled.timestamp`
|
|
* is milliseconds-since-epoch on Hermes/React Native (jsinspector-modern's
|
|
* getTimestampMs is std::chrono::duration<double, std::milli>), exactly as on
|
|
* Chrome. The RN blueprint used to do `new Date(entry.timestamp * 1000)`, which
|
|
* assumed seconds and stamped every flat-log line with a year-58473 date. Both
|
|
* blueprints now share this helper so their log files agree and cannot re-diverge.
|
|
*/
|
|
describe("consoleTimestampToIso", () => {
|
|
it("treats the CDP timestamp as milliseconds and preserves the instant", () => {
|
|
// A realistic Hermes/Chrome value: system_clock::now() in ms, i.e. Date.now().
|
|
const ms = 1_783_000_000_000; // 2026-07-01T...Z
|
|
const iso = consoleTimestampToIso(ms);
|
|
expect(iso).toBe(new Date(ms).toISOString());
|
|
// Round-trips to the exact same instant — no scaling.
|
|
expect(new Date(iso).getTime()).toBe(ms);
|
|
// The current-millennium year, NOT the year-58473 corruption.
|
|
expect(new Date(iso).getUTCFullYear()).toBe(2026);
|
|
});
|
|
|
|
it("does NOT reintroduce the * 1000 corruption (far-future year)", () => {
|
|
// Guards the exact bug: the pre-fix `new Date(ms * 1000)` interpreted a real
|
|
// ms timestamp as seconds and landed ~56000 years out — `new Date(ms * 1000)`
|
|
// below is year 58471 for this input (the reviewer observed 58473 on-device
|
|
// with a slightly larger clock). The helper must keep a real ms value in the
|
|
// current millennium; re-adding `* 1000` inside it fails this assertion.
|
|
const ms = 1_783_000_000_000;
|
|
expect(new Date(consoleTimestampToIso(ms)).getUTCFullYear()).toBeLessThan(3000);
|
|
});
|
|
|
|
it("round-trips a live Date.now() to the current year", () => {
|
|
const now = Date.now();
|
|
const iso = consoleTimestampToIso(now);
|
|
expect(new Date(iso).getTime()).toBe(now);
|
|
expect(new Date(iso).getUTCFullYear()).toBe(new Date(now).getUTCFullYear());
|
|
});
|
|
|
|
it("coerces an unrepresentable timestamp to a valid ISO string instead of throwing", () => {
|
|
// new Date(x).toISOString() throws RangeError not only for non-finite x but
|
|
// also for a FINITE value outside Date's ±8.64e15 ms range (8.7e15 passes
|
|
// Number.isFinite yet still throws). Inside the typed-emitter listener that
|
|
// throw would silently drop the log entry, so the helper must coerce all of
|
|
// these to "now" and never throw.
|
|
for (const bad of [NaN, Infinity, -Infinity, 8.7e15, -8.7e15, 1e300]) {
|
|
const before = Date.now();
|
|
const iso = consoleTimestampToIso(bad);
|
|
const after = Date.now();
|
|
const t = new Date(iso).getTime();
|
|
expect(Number.isNaN(t)).toBe(false);
|
|
// Fell back to "now".
|
|
expect(t).toBeGreaterThanOrEqual(before);
|
|
expect(t).toBeLessThanOrEqual(after);
|
|
}
|
|
});
|
|
});
|