Skip to content

fix(stdlib): #4917 — real semantics (or loud warn) for silently-ignored adapter options and no-ops#4996

Merged
proggeramlug merged 2 commits into
mainfrom
fix/stdlib-adapter-noops-4917
Jun 11, 2026
Merged

fix(stdlib): #4917 — real semantics (or loud warn) for silently-ignored adapter options and no-ops#4996
proggeramlug merged 2 commits into
mainfrom
fix/stdlib-adapter-noops-4917

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Closes #4917 (stub-elimination epic #4919). Code-only per contributor convention — no version bump / changelog; maintainer folds metadata at merge.

Per-checkbox status, in the issue's order:

Options accepted but ignored

  • zlib stream factoriesoptions.level is now honored by every deflate-family factory (createGzip/Deflate/DeflateRaw/...), threaded through make_codec_state; .reset() rebuilds the codec at the stream's level. deflateRawSync gains its missing options arg (stdlib fn + codegen native-table arity + FFI decl + dynamic-dispatch arm). A supplied dictionary now warns once instead of silently mis-compressing — the warn lives in the shared js_zlib_validate_options (perry-runtime), so the ext-zlib auto-optimize path inherits it. Brotli/zstd factories warn once when an options object is passed (params shape still unwired). Remaining honest gaps (strategy/memLevel validated-but-unapplied, brotli/zstd params) keep narrowed manifest notes.
  • http.Agent — partly stale: on the default ext-http path, per-agent reqwest::Client pooling already honors keepAlive/maxFreeSockets/keepAliveMsecs, and destroy() already drops the cached client (real pool teardown) — un-flagged in the manifest. The true remaining no-ops, keepSocketAlive/reuseSocket, now warn once on both the ext and stdlib paths.
  • exponential-backoff — full option surface honored: numOfAttempts, startingDelay, timeMultiple, maxDelay, delayFirstAttempt, jitter: 'full', and the retry(e, attemptNumber) predicate. Bigger find: the adapter never retried async tasks at all (the first Promise was passed through verbatim). Rewritten as a GC-rooted state machine that attaches reactions via js_promise_then and schedules delays through the timer queue — retry-on-rejection with no blocking thread::sleep on the main thread. The old code also read options from the wrong register (declared *const ObjectHeader for an NA_F64 arg).

No-ops returning success

  • fastify upgrade — registering an upgrade handler now warns once at on() time (the stored-callback silence was the lie); upgrade requests keep the loud 501 until FastifyInstance.server returns a number — blocks ws/http2 upgrade piggybacking #1113 lands real dispatch.
  • worker.ref()/unref() — already real: the cited worker_threads.rs:452,638,643 are MessagePort no-ops, not Worker. Worker.ref/unref flip WorkerRecord.refed, which gates js_worker_threads_has_pending. Verified behaviorally both directions (unref'd live worker → process exits in 0.46s; refed worker → holds until terminate()). Stale manifest stub notes dropped.

Wrong-shaped results

  • mongodb findOne() — resolves a parsed document object (doc.field works). The blocking JSON.parse(...).foo → NaN bug noted in the code comment is fixed on main (probed). BSON-specific types surface in relaxed extended-JSON shape (_id.$oid). Parse happens in the main-thread converter (no worker-arena hazard).
  • mysql2 column metadatafield.type/columnType now carry the numeric MySQL wire type ID via a name→ID map (sqlx 0.8 keeps the raw ColumnType byte pub(crate); the name strings are a bijection over type+flags). length stays 0 and is documented — max_size is not reachable through sqlx's public API. Both perry-stdlib/mysql2 and perry-ext-mysql2 updated.
  • pg column metadatadataTypeID is the real numeric type OID (PgTypeInfo::oid()), tableID/columnID come from the RowDescription via relation_id()/relation_attribute_no() (0 for expression columns, like Node), plus dataTypeSize/dataTypeModifier (-1 sentinel) and format: "text" for node-pg shape compat; field objects now carry a keys array so by-name access works. Both stdlib and ext twins updated.

Manifest / docs

  • #4917 stub inventory 18 → 9 (remaining: 3 compressor factories partial-strategy + 4 brotli/zstd params + 2 Agent socket hooks), with per-entry notes reworded to describe the remaining divergence. stub_inventory.rs keystone spot-checks updated (findOne/backOff/ref/unref/destroy moved to must-NOT-be-stub). perry.d.ts / reference.md regenerated via scripts/regen_api_docs.sh.

Verification

  • New test-files/test_gap_zlib_4917_level.ts: byte-for-byte PARITY-MATCH vs node v26 (level honored on streams + deflateRawSync, RangeError on level 99, round-trips).
  • exponential-backoff e2e: retry predicate called with attempt numbers, success after N rejections, retry: false stops after 1 call, numOfAttempts: 3 exhausts after exactly 3 calls rejecting with the last error, delays measured.
  • Worker ref/unref e2e both directions (above).
  • dictionary warn-once verified on stderr.
  • 0-regress: test_parity_zlib, test_gap_zlib_fs_assert_2935_2752_2971, test_parity_worker_threads MATCH vs node; test_gap_zlib_3285_params diff is pre-existing (byte-identical output from a pre-change baseline binary — the ext-zlib .params() flip isn't active in this environment).
  • cargo test -p perry-api-manifest (36 green incl. stub inventory drift guard), ext-http/-mysql2/-pg unit tests (28 green), full release build of all touched crates green, file-size gate green.

Ralph Küpper added 2 commits June 11, 2026 13:04
… options and no-ops (#4917)

zlib: deflate-family stream factories + deflateRawSync honor options.level
(threaded through make_codec_state; .reset() rebuilds at the same level);
a supplied dictionary warns once via the shared runtime validator (covers
ext-zlib too); Brotli/zstd factories warn once when an options object is
passed.

exponential-backoff: real npm semantics — numOfAttempts/startingDelay/
timeMultiple/maxDelay/delayFirstAttempt/jitter/retry parsed and honored,
and Promise-returning tasks now retry on REJECTION (previously the first
promise was passed through and no retry ever happened) via a GC-rooted
state machine chaining js_promise_then + timer-queue delays instead of
blocking thread::sleep.

mongodb: findOne resolves a parsed document object (the JSON.parse
property-access bug that blocked this is fixed); BSON types surface in
relaxed extended-JSON shape.

mysql2/pg: FieldPacket.type/columnType carry the numeric MySQL wire type
ID (name->ID map; sqlx 0.8 keeps the raw byte pub(crate)); pg fields get
numeric dataTypeID (type OID), tableID/columnID from the RowDescription
via sqlx relation_id()/relation_attribute_no(), dataTypeSize/-Modifier
sentinel -1, format "text". Both stdlib and ext twins updated.

http.Agent: keepSocketAlive/reuseSocket warn once (reqwest owns the
pool); destroy() un-flagged — it really drops the per-agent client on
the ext path. fastify: storing an 'upgrade' handler warns at
registration (#1113 tracks real dispatch). worker.ref/unref: already
real (event-loop refcount verified both directions); stale stub notes
dropped — the lines #4917 cited are MessagePort no-ops, not Worker.

Manifest: #4917 stub inventory 18 -> 9 with narrowed notes; keystone
test updated; docs regenerated. New parity test
test_gap_zlib_4917_level.ts matches node v26 byte-for-byte.
da4f8be (#4994) landed with a red lint job (admin merge), so every
PR's merge-ref now fails it. Two findings, both from that commit:

- array/sort.rs:126 — the second tail-copy loop's store sits one line
  outside the ±6-line window of the existing GC_STORE_AUDIT(STACK)
  marker; add the same marker next to that loop (same caller-rooted
  scratch-buffer justification).
- array/from_concat.rs:174 + object/prototype_chain.rs:58 — new
  GcHeader probes outside addr_class.rs (from_concat is the #4994 split
  of the already-allowlisted concat_reverse probe; prototype_chain's is
  guarded by is_above_handle_band + is_valid_obj_ptr). Allowlist both
  with justifications.
@proggeramlug proggeramlug force-pushed the fix/stdlib-adapter-noops-4917 branch from 9e7ef0e to 60bc981 Compare June 11, 2026 11:14
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and added a second commit unbreaking the lint job: #4994 (da4f8be) was admin-merged with lint red, so every PR merge-ref inherited two audit failures from it — a GC_STORE_AUDIT marker one line out of window in array/sort.rs, and two un-allowlisted GcHeader probes (array/from_concat.rs, object/prototype_chain.rs). All four lint gates (fmt, file-size, GC store-site, addr-class) verified green locally on the rebased tree.

@proggeramlug proggeramlug merged commit 5ce0d67 into main Jun 11, 2026
12 of 13 checks passed
@proggeramlug proggeramlug deleted the fix/stdlib-adapter-noops-4917 branch June 11, 2026 11:15
proggeramlug pushed a commit that referenced this pull request Jun 11, 2026
…ispatch arity

#4996 grew the zlib deflateRawSync dispatch entry a 2nd (options) slot when it
made the `level` option real (NATIVE_MODULE_TABLE: [NA_F64, NA_F64]) but left
the API manifest declaring a single param, tripping perry-codegen's
manifest_param_counts_match_dispatch_table consistency test (cargo-test gate red
on main). Declare the optional options param, mirroring deflateSync/gzipSync,
and regenerate the API docs (.d.ts) so api-docs-drift stays green.
proggeramlug pushed a commit that referenced this pull request Jun 12, 2026
…tch dispatch table

The #4996 zlib options work widened the NATIVE_MODULE_TABLE entry to
(NA_F64, NA_F64) — data plus the { level } options object, matching Node's
deflateRawSync(buffer[, options]) — but the API manifest still declared a
single string param. manifest_param_counts_match_dispatch_table has been
red on main since (hidden behind the node_stream fail-fast in CI logs).
Mirror deflateSync's manifest shape and regenerate perry.d.ts.
proggeramlug pushed a commit that referenced this pull request Jun 12, 2026
…tch dispatch table

The #4996 zlib options work widened the NATIVE_MODULE_TABLE entry to
(NA_F64, NA_F64) — data plus the { level } options object, matching Node's
deflateRawSync(buffer[, options]) — but the API manifest still declared a
single string param. manifest_param_counts_match_dispatch_table has been
red on main since (hidden behind the node_stream fail-fast in CI logs).
Mirror deflateSync's manifest shape and regenerate perry.d.ts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant