Skip to content

implement RPC support for inbound UDP connect() event - #7433

Open
ThomasRubini wants to merge 3 commits into
mainfrom
trubini/udp_workers_rpc
Open

ThomasRubini wants to merge 3 commits into
mainfrom
trubini/udp_workers_rpc

Conversation

@ThomasRubini

Copy link
Copy Markdown
Collaborator

This implements RPC support for the new UDP connect custom event, so that we can call it from edgeworker.

Please note that EVENT_TYPE's value may change after approval depending on the feedback I'll get on the internal PR.

@ThomasRubini
ThomasRubini marked this pull request as ready for review September 18, 2026 19:24
@ThomasRubini
ThomasRubini requested review from a team as code owners September 18, 2026 19:24
@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

UdpConnectCustomEvent::sendRpc() now calls EventDispatcher.udpConnect(), but Server::WorkerdBootstrapImpl::EventDispatcherImpl never overrides that method. The generated default rejects the request, so an edgeworker cannot deliver the UDP flow. Add the endpoint near jsRpcSession() and retain the worker while receiveRpc() awaits the handler:

    kj::Promise<void> udpConnect(UdpConnectContext context) override {
      auto worker = getWorker();
      auto& workerRef = *worker;
      return api::UdpConnectCustomEvent::receiveRpc(context, workerRef).attach(kj::mv(worker));
    }

@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

PR #7433 adds bidirectional Cap'n Proto RPC transport for inbound UDP connect() events.

  1. P1: The server dispatcher never implements udpConnect, so remote calls are rejected by the generated default handler. Posted a suggested fix: implement RPC support for inbound UDP connect() event #7433 (comment)

Limerick incoming!
A UDP flow sought its way,
But dispatch never joined the relay.
With a handler in line,
The packets align,
And the endpoint can answer all day.

github run

@codecov-commenter

codecov-commenter commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.51462% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.04%. Comparing base (b93fa71) to head (18de5f2).

Files with missing lines Patch % Lines
src/workerd/api/sockets.c++ 63.49% 35 Missing and 11 partials ⚠️
src/workerd/server/server-test.c++ 97.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7433      +/-   ##
==========================================
+ Coverage   37.96%   38.04%   +0.07%     
==========================================
  Files         836      836              
  Lines      257859   258027     +168     
  Branches    20397    20414      +17     
==========================================
+ Hits        97908    98157     +249     
+ Misses     148370   148258     -112     
- Partials    11581    11612      +31     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

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.

2 participants