Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail WorkerEntrypoint::connect with a JSG error #640

Merged
merged 1 commit into from
May 17, 2023

Conversation

dom96
Copy link
Collaborator

@dom96 dom96 commented May 11, 2023

Test Plan

$ bazel run @workerd//src/workerd/server:workerd -- serve $PWD/deps/workerd/samples/tcp/config.capnp --watch --verbose --experimental

Verified that the gopher example still works with proxy.

@dom96 dom96 requested a review from jasnell May 11, 2023 16:39
src/workerd/api/actor.h Outdated Show resolved Hide resolved
src/workerd/api/http.c++ Outdated Show resolved Hide resolved
@bcaimano bcaimano self-requested a review May 11, 2023 18:37
@bcaimano
Copy link
Contributor

(Tagging myself as a DO reviewer, looks pretty reasonable so I doubt I'll be much more than a green check.)

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as is but would definitely prefer the bool args be changed to an options struct. Not going to block on it tho

@kentonv
Copy link
Member

kentonv commented May 12, 2023

Why is this needed? Won't the receiving end refuse the connection for now? And eventually we'll actually support this, when we support receiving connections in workers, right?

If this needs to be disabled on DO stubs then I would think it also needs to be disabled on service bindings. But I don't see the need, I think we should allow it.

@dom96 dom96 force-pushed the dominik/disable-connect-on-do branch from 4d25122 to 690e162 Compare May 17, 2023 11:33
@dom96 dom96 force-pushed the dominik/disable-connect-on-do branch from 690e162 to cc029e9 Compare May 17, 2023 11:45
@dom96 dom96 changed the title Disables connect on a Durable Object Fetcher. Fail WorkerEntrypoint::connect with a JSG error May 17, 2023
@dom96 dom96 merged commit 490ca04 into main May 17, 2023
7 checks passed
@dom96 dom96 deleted the dominik/disable-connect-on-do branch May 17, 2023 14:59
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.

None yet

5 participants