test(hermes-base): isolate late errors and preserve spacing semantics

Author: github-actions[bot]

src/utils/hermes-base.ts

@@ -1150,21 +1150,31 @@ export function normalizeDisassemblyLine(
   return line;
 }
 
-/** Only formatting outside quoted operands may be collapsed. */
+/**
+ * Collapse formatting outside quoted operands without altering literal code units.
+ * ASCII dump spacing takes the char-code fast path; uncommon Unicode characters
+ * retain the complete runtime `\s` semantics instead of a narrower space list.
+ */
 function normalizeOperandSpacing(text: string): string {
   let result = '';
   let quoted = false;
   let escaped = false;
   let spacing = false;
-  for (const char of text) {
+  for (let i = 0; i < text.length; i++) {
+    const char = text[i];
     if (quoted) {
       result += char;
       if (escaped) escaped = false;
       else if (char === '\\') escaped = true;
       else if (char === '"') quoted = false;
       continue;
     }
-    if (/\s/.test(char)) {
+    const code = text.charCodeAt(i);
+    const whitespace =
+      code <= 0x7f
+        ? isSpace(code) || (code >= 0x0a && code <= 0x0c)
+        : /\s/.test(char);
+    if (whitespace) {
       if (!spacing) result += ' ';
       spacing = true;
     } else {
@@ -1343,6 +1353,7 @@ class DumpReader {
     });
   }
 
+  /** Consume lookahead first, then the stream; null is reserved for EOF. */
   private async nextLine(): Promise<string | null> {
     if (this.pending !== null) {
       const line = this.pending;

src/utils/hermes-raw.ts

@@ -16,13 +16,15 @@ import { type LiteralBuffers, LiteralResolver } from './hermes-literals';
 
 export class UnverifiableHermesBytecode extends Error {}
 
+/** Fail closed on missing references rather than comparing equal placeholders. */
 function requireValue<T>(value: T | null | undefined, what: string): T {
   if (value === null || value === undefined) {
     throw new UnverifiableHermesBytecode(what);
   }
   return value;
 }
 
+/** Bound a reference to its owning section before creating a zero-copy view. */
 function checkedSlice(data: Buffer, start: number, length: number): Buffer {
   if (
     !Number.isSafeInteger(start) ||
@@ -248,10 +250,15 @@ interface RawInstruction {
   size: number;
 }
 
+/** Remove encoding-width suffixes; operand values remain part of the audit. */
 function foldWidth(opcode: string): string {
   return opcode.replace(/(?:LongIndex|Long|Short)$/, '');
 }
 
+/**
+ * Read raw operand boundaries and verify integer values against the HBC bytes.
+ * Doubles use their exact bits, not the rounded number printed by hermesc.
+ */
 function parseRawInstruction(
   line: string,
   data: HermesSemanticData,
@@ -460,6 +467,7 @@ export interface RawAuditResult {
   detail?: string;
 }
 
+/** Decode split UTF-8 sequences without buffering the whole raw dump. */
 async function* linesOf(stream: NodeJS.ReadableStream): AsyncGenerator<string> {
   const decoder = new StringDecoder('utf8');
   let rest = '';
@@ -476,7 +484,10 @@ async function* linesOf(stream: NodeJS.ReadableStream): AsyncGenerator<string> {
   if (rest) yield rest;
 }
 
-/** A second, raw pass: no additional compile, and only one function in memory. */
+/**
+ * Audit a pair of raw dumps while retaining one function per side plus HBC data.
+ * Cancellation/errors fail closed; both subprocesses are terminated and reaped.
+ */
 export async function auditRawHermesBytecode(
   command: string,
   files: [string, string],

tests/fixtures/hermes-async-check.cjs

@@ -0,0 +1,61 @@
+/**
+ * Run async-error regressions outside the test runner under either Bun or Node.
+ * Listeners remain installed until natural exit: even errors after the result
+ * marker make the child fail. A parent deadline also detects leaked handles.
+ */
+const assert = require('node:assert/strict');
+const path = require('node:path');
+
+const unexpected = [];
+/** Record both Promise rejections and uncaught ChildProcess error events. */
+function recordUnexpected(kind, error) {
+  unexpected.push(kind);
+  process.exitCode = 1;
+  console.error(`HERMES_ASYNC_ERROR ${kind}: ${String(error)}`);
+}
+process.on('unhandledRejection', (error) =>
+  recordUnexpected('unhandledRejection', error),
+);
+process.on('uncaughtException', (error) =>
+  recordUnexpected('uncaughtException', error),
+);
+
+/** Exercise the actual API, then allow timers and immediates to report errors. */
+async function main() {
+  const config = JSON.parse(process.argv[2]);
+  if (config.cwd) process.chdir(config.cwd);
+  let result;
+  if (config.operation === 'abort') {
+    const { compareHermesBytecode } = require(path.resolve(config.modulePath));
+    const controller = new AbortController();
+    controller.abort();
+    result = await compareHermesBytecode(
+      config.command || process.execPath,
+      'missing-a',
+      'missing-b',
+      { signal: controller.signal, timeoutMs: 500 },
+    );
+    assert.equal(result.status, 'dump-failed');
+    assert.match(result.detail, /abort/i);
+  } else if (config.operation === 'compile') {
+    const { compileHermesByteCode } = require(path.resolve(config.modulePath));
+    result = await compileHermesByteCode(config.options);
+  } else if (config.operation === 'control-rejection') {
+    setImmediate(() => Promise.reject(new Error('intentional rejection')));
+  } else if (config.operation === 'control-exception') {
+    setImmediate(() => {
+      throw new Error('intentional exception');
+    });
+  } else {
+    throw new Error(`Unknown operation: ${config.operation}`);
+  }
+  // Checking only immediately after await can miss later-turn error delivery.
+  await new Promise((resolve) => setTimeout(resolve, 0));
+  await new Promise((resolve) => setImmediate(resolve));
+  assert.deepEqual(unexpected, []);
+  console.log(`HERMES_ASYNC_RESULT ${JSON.stringify(result)}`);
+}
+main().catch((error) => {
+  console.error(error);
+  process.exitCode = 1;
+});

tests/hermes-base-safety.test.ts

@@ -31,3 +31,61 @@ describe('Hermes-base normalization must retain semantic differences', () => {
     );
   });
 });
+
+/** Previous spacing implementation, kept as an independent compatibility oracle. */
+function referenceSpacing(text: string): string {
+  let result = '';
+  let quoted = false;
+  let escaped = false;
+  let spacing = false;
+  for (const char of text) {
+    if (quoted) {
+      result += char;
+      if (escaped) escaped = false;
+      else if (char === '\\') escaped = true;
+      else if (char === '"') quoted = false;
+      continue;
+    }
+    if (/\s/.test(char)) {
+      if (!spacing) result += ' ';
+      spacing = true;
+    } else {
+      result += char;
+      spacing = false;
+      if (char === '"') quoted = true;
+    }
+  }
+  return result;
+}
+
+describe('Hermes operand spacing compatibility', () => {
+  test('matches the previous whitespace behavior for every UTF-16 code unit', () => {
+    const mismatches: number[] = [];
+    for (let code = 0; code <= 0xffff; code++) {
+      const operands = ` r0, ${String.fromCharCode(code)} r1`;
+      if (
+        normalize(`    Mov${operands}`) !==
+        `    Mov${referenceSpacing(operands)}`
+      ) {
+        mismatches.push(code);
+      }
+    }
+    expect(mismatches).toEqual([]);
+  });
+
+  test('preserves whitespace, escapes and surrogate pairs inside quotes', () => {
+    const whitespace =
+      '\t\n\v\f\r \u00a0\u1680\u2000\u2028\u2029\u202f\u205f\u3000\ufeff';
+    const operands = [
+      ` r0, "a${whitespace}b"`,
+      String.raw` r0, "a\"  b\\  c",  r1`,
+      ` r0, "😀  𠮷\ud800\udfff",\u00a0\ufeffr1`,
+      ' r0, \u0085\u180e\u200b r1',
+    ];
+    for (const operand of operands) {
+      expect(normalize(`    Mov${operand}`)).toBe(
+        `    Mov${referenceSpacing(operand)}`,
+      );
+    }
+  });
+});

tests/hermes-compile.test.ts

@@ -285,30 +285,40 @@ exec "${hermesc}" "$@"
     );
     const map = path.join(outputFolder, `${bundleName}.map`);
     fs.writeFileSync(map, '{}');
-    const cwd = process.cwd();
-    const unhandled: unknown[] = [];
-    const onUnhandled = (error: unknown) => unhandled.push(error);
-    process.on('unhandledRejection', onUnhandled);
-    process.chdir(dir);
-    try {
-      const result = await compileHermesByteCode({
-        bundleName,
-        outputFolder,
-        sourcemapOutput: map,
-        shouldCleanSourcemap: true,
-        baseRequest: { option: baseHbc, verify: true },
-        hermesCommand: wrapper,
-      });
-      expect(fs.existsSync(marker)).toBe(true);
-      expect(result.outcome).toBe('dump-failed');
-      expect(result.base).toBeNull();
-      expect(unhandled).toEqual([]);
-      expect(fs.readFileSync(map, 'utf8')).toBe('{}');
-      expect(leftovers()).toEqual([]);
-    } finally {
-      process.chdir(cwd);
-      process.off('unhandledRejection', onUnhandled);
-    }
+    const child = spawnSync(
+      process.execPath,
+      [
+        path.join(__dirname, 'fixtures/hermes-async-check.cjs'),
+        JSON.stringify({
+          operation: 'compile',
+          modulePath: require.resolve('../src/bundle-runner'),
+          cwd: dir,
+          options: {
+            bundleName,
+            outputFolder,
+            sourcemapOutput: map,
+            shouldCleanSourcemap: true,
+            baseRequest: { option: baseHbc, verify: true },
+            hermesCommand: wrapper,
+          },
+        }),
+      ],
+      { encoding: 'utf8', timeout: 4000 },
+    );
+    expect(child.error).toBeUndefined();
+    expect(child.signal).toBeNull();
+    expect(child.status).toBe(0);
+    expect(child.stderr).not.toContain('HERMES_ASYNC_ERROR');
+    const line = child.stdout
+      .split('\n')
+      .find((value) => value.startsWith('HERMES_ASYNC_RESULT '));
+    expect(line).toBeDefined();
+    const result = JSON.parse(line!.slice('HERMES_ASYNC_RESULT '.length));
+    expect(fs.existsSync(marker)).toBe(true);
+    expect(result.outcome).toBe('dump-failed');
+    expect(result.base).toBeNull();
+    expect(fs.readFileSync(map, 'utf8')).toBe('{}');
+    expect(leftovers()).toEqual([]);
   }, 5000);
 
   test('a selection started ahead of time is consumed by the compile', async () => {

tests/hermes-timeout.test.ts

@@ -1,4 +1,5 @@
 import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
+import { spawnSync } from 'child_process';
 import fs from 'fs-extra';
 import os from 'os';
 import path from 'path';
@@ -85,18 +86,45 @@ describe.if(process.platform !== 'win32')('Hermes subprocess deadlines', () => {
     }
   }, 2000);
 
-  test('an already-aborted request fails without an unhandled spawn error', async () => {
-    const controller = new AbortController();
-    controller.abort();
-    const result = await compareHermesBytecode(
-      hanging,
-      'missing-a',
-      'missing-b',
-      {
-        signal: controller.signal,
-        timeoutMs: 500,
-      },
+  test('an already-aborted request has no late unhandled errors in an isolated process', () => {
+    const child = spawnSync(
+      process.execPath,
+      [
+        path.join(__dirname, 'fixtures/hermes-async-check.cjs'),
+        JSON.stringify({
+          operation: 'abort',
+          modulePath: require.resolve('../src/utils/hermes-base'),
+          command: hanging,
+        }),
+      ],
+      { encoding: 'utf8', timeout: 1500 },
     );
-    expect(result.status).toBe('dump-failed');
+    expect(child.error).toBeUndefined();
+    expect(child.signal).toBeNull();
+    expect(child.status).toBe(0);
+    expect(child.stderr).not.toContain('HERMES_ASYNC_ERROR');
+    expect(child.stdout).toContain('"status":"dump-failed"');
   }, 2000);
 });
+
+describe('isolated async-error observer negative controls', () => {
+  for (const [operation, event] of [
+    ['control-rejection', 'unhandledRejection'],
+    ['control-exception', 'uncaughtException'],
+  ]) {
+    test(`fails for a late ${event}`, () => {
+      const child = spawnSync(
+        process.execPath,
+        [
+          path.join(__dirname, 'fixtures/hermes-async-check.cjs'),
+          JSON.stringify({ operation }),
+        ],
+        { encoding: 'utf8', timeout: 1500 },
+      );
+      expect(child.error).toBeUndefined();
+      expect(child.signal).toBeNull();
+      expect(child.status).toBe(1);
+      expect(child.stderr).toContain(`HERMES_ASYNC_ERROR ${event}`);
+    }, 2000);
+  }
+});