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

fix: restart the dev proxy server whenever it closes #317

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

threepointone
Copy link
Contributor

When we run wrangler dev, the session that we setup with the preview endpoint doesn't last forever, it dies after ignoring it for 5-15 minutes or so. The fix for this is to simply reconnect the server. So we use a state hook as a sigil, and add it to the dependency array of the effect that sets up the server, and simply change it every time the server closes.

Fixes #197

(In wrangler1, we used to restart the whole process, including uploading the worker again, making a new preview token, and so on. It looks like that they may not have been necessary.)

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2022

🦋 Changeset detected

Latest commit: 0343636

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Not having any way of automatically testing this, I assume that you have done exhaustive manual testing ;-) I can't think of any reason why simply recreating the remote client would not work.

What happens if you are in the middle of debugging? What is the developer experience? Does the Worker appear to crash and restart?

I left a couple of NIT comments, which you can consider.

packages/wrangler/src/inspect.ts Outdated Show resolved Hide resolved
packages/wrangler/src/proxy.ts Outdated Show resolved Hide resolved
@@ -119,6 +135,11 @@ export function usePreviewServer({
const remote = connect(`https://${previewToken.host}`);
cleanupListeners.push(() => remote.destroy());

// As mentioned above, the session may die at any point,
// so we need to restart the effect.
remote.on("close", retryServerSetup);
Copy link
Contributor

Choose a reason for hiding this comment

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

Very clean solution.

When we run `wrangler dev`, the session that we setup with the preview endpoint doesn't last forever, it dies after ignoring it for 5-15 minutes or so. The fix for this is to simply reconnect the server. So we use a state hook as a sigil, and add it to the dependency array of the effect that sets up the server, and simply change it every time the server closes.

Fixes #197

(In wrangler1, we used to restart the whole process, including uploading the worker again, making a new preview token, and so on. It looks like that they may not have been necesssary.)
@threepointone
Copy link
Contributor Author

We don't do breakpoint debugging at the moment, so there's no crash visible. I assume we'll have to work on this after we land breakpoints in the debugger. One thing that I think will break, is websocket connections. But that's possible even without dev, and folks should write code that's resilient to websockets dropping and reconnecting.

We'll probably have to iterate on this more in the future when we're working on developing multiple workers, but this is a good place for now, at least for parity with wrangler 1.

@threepointone threepointone merged commit d6ef61a into main Jan 27, 2022
@threepointone threepointone deleted the restart-expired-sessions branch January 27, 2022 15:13
@github-actions github-actions bot mentioned this pull request Jan 27, 2022
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.

restart dev when session expires
2 participants