Skip to content

Commit

Permalink
Adding abort to the waitForPortToBeAvailable as an option and in th…
Browse files Browse the repository at this point in the history
…e `useEffect` the abort signal is called in the return which allows for graceful closing with no hanging

of the `waitForPortToBeAvailable`

fixes #375
  • Loading branch information
JacobMGEvans committed Mar 31, 2022
1 parent 1ca3092 commit c0eba4c
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 13 deletions.
10 changes: 10 additions & 0 deletions .changeset/wet-tables-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"wrangler": patch
---

fix: abort async operations in the `Remote` component to avoid unwanted side-effects
When the `Remote` component is unmounted, we now signal outstanding `fetch()` requests, and
`waitForPortToBeAvailable()` tasks to cancel them. This prevents unexpected requests from appearing
after the component has been unmounted, and also allows the process to exit cleanly without a delay.

fixes #375
14 changes: 7 additions & 7 deletions packages/wrangler/src/create-worker-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface CfPreviewToken {
async function sessionToken(
account: CfAccount,
ctx: CfWorkerContext,
abort: AbortSignal
abortSignal: AbortSignal
): Promise<CfPreviewToken> {
const { accountId } = account;
const initUrl = ctx.zone
Expand All @@ -67,7 +67,7 @@ async function sessionToken(

const { exchange_url } = await fetchResult<{ exchange_url: string }>(initUrl);
const { inspector_websocket, token } = (await (
await fetch(exchange_url, { signal: abort })
await fetch(exchange_url, { signal: abortSignal })
).json()) as { inspector_websocket: string; token: string };
const { host } = new URL(inspector_websocket);
const query = `cf_workers_preview_token=${token}`;
Expand Down Expand Up @@ -98,12 +98,12 @@ async function createPreviewToken(
account: CfAccount,
worker: CfWorkerInit,
ctx: CfWorkerContext,
abort: AbortSignal
abortSignal: AbortSignal
): Promise<CfPreviewToken> {
const { value, host, inspectorUrl, prewarmUrl } = await sessionToken(
account,
ctx,
abort
abortSignal
);

const { accountId } = account;
Expand Down Expand Up @@ -159,12 +159,12 @@ export async function createWorkerPreview(
init: CfWorkerInit,
account: CfAccount,
ctx: CfWorkerContext,
abort: AbortSignal
abortSignal: AbortSignal
): Promise<CfPreviewToken> {
const token = await createPreviewToken(account, init, ctx, abort);
const token = await createPreviewToken(account, init, ctx, abortSignal);
const response = await fetch(token.prewarmUrl.href, {
method: "POST",
signal: abort,
signal: abortSignal,
});
if (!response.ok) {
// console.error("worker failed to prewarm: ", response.statusText);
Expand Down
8 changes: 7 additions & 1 deletion packages/wrangler/src/dev/local.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,16 @@ function useLocalWorker({
const removeSignalExitListener = useRef<() => void>();
const [inspectorUrl, setInspectorUrl] = useState<string | undefined>();
useEffect(() => {
const abortController = new AbortController();
async function startLocalWorker() {
if (!bundle || !format) return;

// port for the worker
await waitForPortToBeAvailable(port, { retryPeriod: 200, timeout: 2000 });
await waitForPortToBeAvailable(port, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
});

if (publicDirectory) {
throw new Error(
Expand Down Expand Up @@ -203,6 +208,7 @@ function useLocalWorker({
});

return () => {
abortController.abort();
if (local.current) {
console.log("⎔ Shutting down local server.");
local.current?.kill();
Expand Down
6 changes: 3 additions & 3 deletions packages/wrangler/src/dev/remote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function useWorker(props: {
const startedRef = useRef(false);

useEffect(() => {
const abortCtrl = new AbortController();
const abortController = new AbortController();
async function start() {
setToken(undefined); // reset token in case we're re-running

Expand Down Expand Up @@ -175,7 +175,7 @@ export function useWorker(props: {
apiToken,
},
{ env: props.env, legacyEnv: props.legacyEnv, zone: props.zone },
abortCtrl.signal
abortController.signal
)
);
}
Expand All @@ -188,7 +188,7 @@ export function useWorker(props: {
});

return () => {
abortCtrl.abort();
abortController.abort();
};
}, [
name,
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,12 @@ export default function useInspector(props: InspectorProps) {
* of the component lifecycle. Convenient.
*/
useEffect(() => {
const abortController = new AbortController();
async function startInspectorProxy() {
await waitForPortToBeAvailable(props.port, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
});
server.listen(props.port);
}
Expand All @@ -167,6 +169,7 @@ export default function useInspector(props: InspectorProps) {
// Also disconnect any open websockets/devtools connections
wsServer.clients.forEach((ws) => ws.close());
wsServer.close();
abortController.abort();
};
}, [props.port, server, wsServer]);

Expand Down
16 changes: 14 additions & 2 deletions packages/wrangler/src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,16 @@ export function usePreviewServer({
// Start/stop the server whenever the
// containing component is mounted/unmounted.
useEffect(() => {
const abortController = new AbortController();
if (proxyServer === undefined) {
return;
}

waitForPortToBeAvailable(port, { retryPeriod: 200, timeout: 2000 })
waitForPortToBeAvailable(port, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
})
.then(() => {
proxyServer.listen(port, ip);
console.log(`⬣ Listening at ${localProtocol}://${ip}:${port}`);
Expand All @@ -304,6 +309,7 @@ export function usePreviewServer({

return () => {
proxyServer.close();
abortController.abort();
};
}, [port, ip, proxyServer, localProtocol]);
}
Expand Down Expand Up @@ -398,9 +404,15 @@ function createStreamHandler(
*/
export async function waitForPortToBeAvailable(
port: number,
options: { retryPeriod: number; timeout: number }
options: { retryPeriod: number; timeout: number; abortSignal: AbortSignal }
): Promise<void> {
return new Promise((resolve, reject) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(options.abortSignal as any).addEventListener("abort", () => {
const error = new Error("");
doReject(error);
});

const timeout = setTimeout(() => {
doReject(new Error(`Timed out waiting for port ${port}`));
}, options.timeout);
Expand Down

0 comments on commit c0eba4c

Please sign in to comment.