Skip to content

fix(jsruntime): #1021 — nestjs sweep silent exit (CJS cycles + process.exit)#1067

Merged
proggeramlug merged 1 commit into
mainfrom
worktree-agent-afb5e65235f1a47b5
May 19, 2026
Merged

fix(jsruntime): #1021 — nestjs sweep silent exit (CJS cycles + process.exit)#1067
proggeramlug merged 1 commit into
mainfrom
worktree-agent-afb5e65235f1a47b5

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Closes the nestjs entry in /tmp/perry-compat-sweep/nestjs from silent exit (empty stdout, inherits TypeError on stderr) to a real HTTP server that boots and responds. Three layered V8-fallback gaps:

  • Template-literal interpolation regression: wrap_commonjs's require-extractor ate every require() that followed a ${...}-containing template literal. Refactored to a stack-aware scan_template_literal that correctly balances interpolated expressions / nested strings / nested templates. Without this, busboy's lib/index.js lost two of its three require() calls and crashed downstream.
  • CJS cycle break: readable-stream's _stream_readable.js / _stream_duplex.js form a 1-hop cycle (one peer top-level-requires the other and uses .prototype immediately at module init via the util.inherits(Duplex, Readable) shape; the other lazy-requires from inside method bodies). The wrap used to hoist all require() into static ESM imports, which turned the lazy CJS edge into a static ESM cycle and made V8 evaluate one peer with the other's bindings in TDZ. Fix:
    • Brace-aware classifier distinguishes top-level requires from those inside function/method/arrow bodies.
    • 1-hop cycle detector: when a relative spec only appears inside a function body, read the peer's source from disk and check whether it requires us back. If yes, skip the static ESM import and resolve at call time.
    • __perry_require_lazy runtime resolver backed by globalThis.__perry_cjs_partial registry that every wrapped module populates with its module object on entry. Lazy requires resolve once the peer's IIFE has finished. Plus auxiliary registration by package name for require(packageName) patterns (NestJS's loadAdapter).
  • process.exit polyfill: NestJS's loadAdapter calls process.exit(1) to bail; the missing polyfill turned the real MODULE_NOT_FOUND into a noisy process.exit is not a function TypeError. Added a log-and-continue shim.

Sweep impact

  • nestjs: silent exit → HTTP server boots, returns 404 for unregistered route. The body= prefix now reaches stdout. Full body=pong parity is gated on Perry-emitted decorator metadata becoming visible to V8-side Reflect.getMetadata (separate work item — Reflect.defineMetadata/getMetadata are not present on Perry-compiled Reflect, so V8's bridge can't see the route registrations).
  • express, hono, debug, dotenv, jose, date-fns, ioredis: all still PASS post-fix (re-verified with the rebuilt binary).

Test plan

  • New end-to-end fixture: test-files/test_issue_1021_cjs_cycle.ts exercises the 1-hop CJS cycle shape and asserts byte-for-byte parity with node --experimental-strip-types.
  • Three new unit tests in crates/perry-jsruntime/src/modules.rs:
    • test_classify_top_level_in_array_literal — busboy's [ require(...), require(...) ] pattern.
    • test_classify_lazy_inside_method_body — readable-stream's lazy require pattern.
    • test_wrap_template_literal_doesnt_eat_following_requires — regression for the busboy scanner bug.
  • All 18 modules::tests::* pass.
  • Compat sweep re-run: express/hono/debug/dotenv/jose/date-fns/ioredis unchanged, nestjs now reaches its main body.

Notes

…s.exit shim

NestJS's silent exit in the compat sweep traced to two layered V8-fallback
gaps. The fix has three parts:

1. **Template-literal scanner**: `wrap_commonjs`'s spec extractor ran a naive
   ``b'`'`` branch that exited template mode at the FIRST `${` and then
   re-entered template mode on the trailing backtick (which is actually the
   close marker, not an open). Consequence: every `require()` after a
   `${...}`-containing template was silently eaten. Busboy's
   `throw new Error(\`Unsupported content type: ${...}\`)` followed by
   `const TYPES = [ require('./types/multipart'), require('./types/urlencoded') ]`
   surfaced this. Replaced with a stack-aware `scan_template_literal` that
   recursively balances `${ ... }` expressions and handles nested strings
   / templates.

2. **CJS cycle break**: `_stream_readable.js` / `_stream_duplex.js` form a
   1-hop cycle — readable lazy-requires duplex from inside method bodies,
   duplex top-level-requires readable and uses `Readable.prototype`
   immediately (the `util.inherits(Duplex, Readable)` pattern). The wrap
   used to hoist ALL `require()` calls into static ESM imports, which
   turned this CJS lazy edge into a static ESM cycle; V8 then evaluated
   one peer with the other's bindings in TDZ and `Readable.prototype`
   came back as `undefined`. Fixed by:
   - Brace-aware classifier (`classify_require_specs`) that distinguishes
     top-level requires from those inside function/method/arrow bodies.
   - 1-hop cycle detector: when a relative spec only appears inside a
     function body, read the peer's source from disk and check whether it
     requires us back. If yes, skip the static ESM import for that spec
     and resolve at call time.
   - Runtime resolver `__perry_require_lazy` backed by a global
     `globalThis.__perry_cjs_partial` registry that every wrapped module
     populates with its `module` object on entry. By the time a lazy
     `require()` actually fires, the peer's IIFE has finished and its
     `module.exports` is fully populated.
   - Auxiliary registration: each module under `node_modules/<pkg>/index.js`
     also registers under the bare package name so NestJS's `loadAdapter`
     pattern (`require(defaultPlatform)` with a runtime string arg) resolves.

3. **`process.exit` polyfill**: NestJS's `loadAdapter` catches the lookup
   error and calls `process.exit(1)` to bail. Our `process` polyfill only
   shipped `env` / `version` / `nextTick` / `stdout` / `stderr`, so the
   `process.exit is not a function` TypeError masked the upstream
   MODULE_NOT_FOUND. Added a polyfill that logs the call and returns,
   letting execution continue so the real error surfaces (or, post-fix,
   so the framework recovers).

Sweep impact:
- nestjs: silent exit -> HTTP server starts, responds with 404 for the
  unregistered route. The `body=` prefix now reaches stdout; full
  decorator-routing parity is gated on Perry-emitted decorator metadata
  becoming visible to V8-side `Reflect.getMetadata` (separate work item).
- express, hono, debug, dotenv, jose, date-fns, ioredis: still PASS
  (re-verified). The classifier's "hoist by default unless cycle"
  heuristic keeps every working sweep package on the eager-static-import
  path it had before.

Test: `test-files/test_issue_1021_cjs_cycle.ts` exercises the 1-hop cycle
shape end-to-end (one peer top-level-requires + uses .prototype, the other
lazy-requires from inside a function body). Byte-for-byte parity with
`node --experimental-strip-types`. Plus three new unit tests on
`wrap_commonjs` / `classify_require_specs` covering busboy's array-literal
top-level requires, the method-body lazy require, and the template-literal
interpolation regression.

No version bump or changelog change (external-contributor convention).
@proggeramlug proggeramlug merged commit 5284700 into main May 19, 2026
9 checks passed
@proggeramlug proggeramlug deleted the worktree-agent-afb5e65235f1a47b5 branch May 19, 2026 06:47
proggeramlug added a commit that referenced this pull request May 19, 2026
…ross-boundary wire (#1117)

Flips the NestJS smoke from `body={"message":"Cannot GET /ping",...}` (404,
no route registered) to `body=pong` (the route is discovered and the
handler runs).

The previous attempt (#1067) wired the Reflect-metadata bridge JS but
NestJS still saw zero modules at scan time. End-to-end the route never
registered because four separate gaps stacked on top of each other:

1. **V8 class wrapper had no methods on the prototype.** `native_class_to_v8`
   returned a `v8::Function` whose `.prototype` was V8's default empty
   object, so `Object.getPrototypeOf(new metatype())[name]` (the path
   NestJS's `MetadataScanner.getAllMethodNames` and `paths-explorer`
   walk) resolved to `undefined`. Now `populate_native_class_v8_prototype`
   mirrors each entry in `CLASS_VTABLE_REGISTRY` onto the wrapper's
   `.prototype` as a `v8::Function` that re-dispatches through the
   vtable's `func_ptr` directly (NOT through `js_native_call_method` —
   that path walks `jsval.as_pointer()` on the receiver, which is junk
   for a V8 object instance and returned the class_id integer instead
   of the method body's return value).

2. **`new metatype()` discarded the prototype chain.** `native_class_constructor`
   used to `retval.set(v8::Object::new(scope))`, overriding the implicit
   `this` whose `[[Prototype]]` V8 already pointed at the populated
   wrapper prototype. Now we return `args.this()` for construct calls.

3. **Class names were lost on the boundary.** `metatype.name` came back
   `""`, which made NestJS's `ModuleTokenFactory` hash every module under
   the same empty-string token — `modules.size` was always 1 (only the
   internal core module won the slot). Added a per-`class_id` →
   `String` registry (`CLASS_NAMES`, populated by codegen via the new
   `js_register_class_name` FFI), and surface it through V8's
   `v8::Function::set_name` so the wrapper carries the user-visible
   class name.

4. **`crypto.createHash().digest()` returned `""`.** The V8-side stub
   threw away `update()` chunks and returned an empty string —
   `ModuleTokenFactory.hashString` hashed every module to the same
   `""`. Added `op_perry_hash` (sha1/sha256/sha384/sha512/md5 via
   `sha1`/`sha2`/`md-5` crates) and rewrote the stub to mirror the
   working `createHmac` path (chunked update → final concat → op
   → encode).

5. **`perf_hooks.performance.now` was undefined.** The default `_ =>
   "export default {}"` fallback applied; NestJS's
   `ModuleTokenFactory.create()` threw `Cannot read properties of
   undefined (reading 'now')` and the dynamic-module compile path
   bailed before any user modules were inserted. Added a real
   `perf_hooks` stub backed by `Date.now()` (good enough for the
   serialization-warning timer NestJS uses it for).

Sub-fixes shipped alongside, all required for the route to land:

- **`reflect-metadata` bridge re-asserted after every module evaluation**
  (`poll_pending_module_evaluations` in `interop.rs`). The npm
  `reflect-metadata` package (loaded transitively by `@nestjs/common`
  via `require("reflect-metadata")`) overwrites `Reflect.defineMetadata`
  /`getMetadata` etc. — without this, our wrappers from `js_runtime_init`
  are gone by the time the first decorator runs. The bridge JS is
  idempotent so re-running on each module evaluation is cheap.
- **Perry closure identity stabilized across V8 crossings**
  (`NATIVE_CLOSURE_HANDLES` cache + `__perry_closure_ptr` property).
  Decorators that key on `descriptor.value` (the method function) now
  see the same `v8::Function` instance as later
  `prototype['methodName']` reads — load-bearing for any
  `WeakMap`-based metadata store on the V8 side.
- **`v8_to_native_metadata_target` recognizes Perry-closure-wrapped V8
  functions** so `descriptor.value` round-trips to the same
  POINTER_TAG | ptr identity Perry uses internally — the precondition
  for the Perry-side `REFLECT_METADATA` store entry from a V8-side
  `Reflect.defineMetadata(...)` to be reachable from a Perry-side
  `Reflect.getMetadata(...)`.

Test: `test-files/test_reflect_metadata.ts` exercises
`defineMetadata`/`getMetadata`/`hasMetadata` + the decorator-factory
shape. NestJS sweep: `body={"message":"Cannot GET /ping",...}` (404)
→ `body=pong`.

Deferred follow-ups:
- Methods that read `this` against a V8-allocated instance still
  dispatch with `TAG_UNDEFINED` as the receiver — the canonical NestJS
  controller-handler shape doesn't use `this` so this matches expected
  behavior for the smoke. Properly wiring instance-as-`this` would
  require allocating a real Perry `ObjectHeader` for V8 wrapper
  instances (or routing through the handle table) — a separate
  follow-up.
- Class-ref id `INT32_TAG | class_id` collision in the bridge
  (existing pre-fix note in `bridge.rs`) is unchanged.

No version bump or changelog change (external-contributor convention).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant