Skip to content

fix: complete 868787448's Bun TCP landing (API docs drift + ext-link shim) - #9780

Open
proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:fix/bun-tcp-landing-followups
Open

fix: complete 868787448's Bun TCP landing (API docs drift + ext-link shim)#9780
proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:fix/bun-tcp-landing-followups

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Two red CI jobs, one cause. 868787448 "feat(bun): add TCP socket facades" (2026-09-03 22:42)
added Bun.listen / Bun.connect but left two derived things un-updated. Both are that
commit's loose ends, so they are fixed together here.

1. check — API docs drift

868787448 added to crates/perry-api-manifest/src/entries/part_4.rs:

method("bun", "listen", false, None),
method("bun", "connect", false, None),

and touched zero files under docs/. The check job's "Check for API docs drift" step
regenerates the docs and diffs, so it has failed ever since:

-Total: 3046 entries across 138 modules.
+Total: 3048 entries across 138 modules.
+- `connect` — module
+- `listen` — module

Regenerated with scripts/regen_api_docs.sh's own commands off a release build
(perry --print-api-manifest=markdown / =dts). The resulting diff is exactly the two
entries CI reported — 3046 → 3048, nothing else. The manifest crate has no target cfgs
(only cfg(test)) and check runs on ubuntu-latest, so this Linux-generated output is
byte-for-byte what CI regenerates.

2. ext-link — undefined symbol, red since 2026-09-04

The reported symbol is js_bun_tcp_listen, but that is a red herring — it is defined
unconditionally at crates/perry-ext-net/src/bun_tcp.rs:275. The actual error is what it
calls:

error: linking with `cc` failed: exit status: 1
  = note: rust-lld: error: undefined symbol: perry_ffi_run_pending
error: could not compile `perry-ext-http` (lib test) due to 1 previous error

js_bun_tcp_listen drives the shared runtime from its bind-poll loop
(perry_ffi::run_pending(2), bun_tcp.rs:324). perry-ext-net supplies a
perry_ffi_run_pending stub in its own test_async_shims.rs — but that module is
#[cfg(test)], so it exists only when perry-ext-net is itself the crate under test, not
when it is linked as an ordinary dependency into perry-ext-http's test binary.
perry-ext-http's shim already covers perry_ffi_spawn_async for exactly this transitive
reason; perry_ffi_run_pending is the same situation one call deeper.

Note this is not the same bug as 6dcd1a598 (#9719), which fixed the analogous
web-fetch-gated HTTP bridge symbols in perry-stdlib and left the TCP path alone.

Verification (linux-x86_64, real builds)

cargo test --release --no-run -p perry-ext-http, with the shim file swapped between main's
version and this one, sharing a target dir so the second run genuinely relinks:

BEFORE (main's shim):  rc=101   Compiling perry-ext-http: 1
    rust-lld: error: undefined symbol: perry_ffi_run_pending
    error: could not compile `perry-ext-http` (lib test) due to 1 previous error

AFTER  (this commit):  rc=0     0 errors
    Executable unittests src/lib.rs (.../perry_ext_http-69d8d87496af011d)

I also linked the ws feature group to check whether perry-ext-ws — which also depends on
perry-ext-net and also lacks the stub — needs the same fix. It does not; both it and
perry-ext-fastify produce executables cleanly, so the fix is deliberately scoped to
perry-ext-http alone rather than added defensively.

Scope

This is one of several independent breakages on main. Also open from the same sweep:
#9776 (warnings — a clashing_extern_declarations compile error, culprit 1ebc65e87)
and #9777 (cargo-test — an unkeyed build-cache env var, culprit 0b68a25cf).

https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2

Summary by CodeRabbit

  • Documentation

    • Updated the API reference and type declarations to include the Bun.connect and Bun.listen methods.
    • Corrected API coverage totals so the published documentation accurately reflects the available Bun networking APIs.
  • Bug Fixes

    • Resolved an issue that prevented the HTTP extension’s test suite from linking successfully.

Ralph Küpper added 3 commits September 5, 2026 07:02
8687874 ("feat(bun): add TCP socket facades") added

    method("bun", "listen", false, None),
    method("bun", "connect", false, None),

to crates/perry-api-manifest/src/entries/part_4.rs but never ran
scripts/regen_api_docs.sh, so the derived docs have been stale since
2026-09-03 and the `check` job fails on its "Check for API docs drift"
step:

    -Total: 3046 entries across 138 modules.
    +Total: 3048 entries across 138 modules.
    +- `connect` — module
    +- `listen` — module

Regenerated with the script's own commands from a release build:
`perry --print-api-manifest=markdown` and `--print-api-manifest=dts`.
The manifest crate carries no target cfgs (only cfg(test)) and the
`check` job runs on ubuntu, so this Linux-generated output is what CI
regenerates.

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 5a3cdb6a-ad00-4cb8-98f6-cfd42fd67fbe

📥 Commits

Reviewing files that changed from the base of the PR and between 12efed1 and e5bc142.

📒 Files selected for processing (4)
  • changelog.d/9780-bun-tcp-landing-followups.md
  • crates/perry-ext-http/src/test_async_shims.rs
  • docs/api/perry.d.ts
  • docs/src/api/reference.md

Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The PR adds a no-op FFI shim for HTTP extension test linking and regenerates API metadata to include Bun.connect and Bun.listen.

Changes

Test linking support

Layer / File(s) Summary
Pending-work test shim
crates/perry-ext-http/src/test_async_shims.rs, changelog.d/9780-bun-tcp-landing-followups.md
The test shim exports perry_ffi_run_pending as a no-op function. The changelog records the linking fix.

Bun API metadata

Layer / File(s) Summary
Bun API declarations and reference
docs/api/perry.d.ts, docs/src/api/reference.md
The generated metadata increases by two entries and lists bun.connect and bun.listen.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e5bc1

This change restores HTTP extension test linking and documents the existing Bun.connect and Bun.listen APIs consistently. No remaining production or user-impacting risk is established.

Suggested reviewers: thehypnoo, jdalton

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the two main fixes: API documentation drift and the external-link test shim. It is specific and related to the changeset.
Description check ✅ Passed The description provides a detailed summary, explains both changes, documents verification commands and results, and states the scope. It does not use the template headings or include the checklist, b…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Conflicts with merge train #9798, which just landed 19 PRs (including #9750's rework of gc_runtime_root_holders.py and a regex.rs split under the 2000-line cap). Could you rebase onto current main? I'd rather you resolve it than have me hand-merge — several of these touch GC root scanning or regex internals where the two changes are independent rewrites of the same code, and that's exactly where a mechanical merge goes quietly wrong. Everything that picked clean is in the next train; I'll pick these up as soon as they rebase.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Held from train127. It cherry-picks with a conflict in test_async_shims.rs (two independent comment blocks explaining different retained externs), and my resolution applied only the changelog fragment without the code commit — verified by git cherry, 2 of 3 commits unapplied. Rather than ship a half-applied PR I have dropped it; a rebase onto current main should make it pick cleanly.

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