From 7d19dd3534b0bb3c3c4dea7558753266089431d5 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Aug 2023 16:16:46 -0500 Subject: [PATCH 1/4] C3: Refactor dns resolution to work better with WARP --- package-lock.json | 36 ++++++++++++- package.json | 2 + .../create-cloudflare/src/helpers/poll.ts | 52 ++++++++++++++----- packages/create-cloudflare/src/types.ts | 13 +++++ 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 37d17f8252d9..9bf4ba9a6968 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,6 +35,8 @@ }, "devDependencies": { "@cloudflare/workers-types": "^4.20221111.1", + "@types/dns2": "^2.0.3", + "dns2": "^2.1.0", "esbuild": "0.16.3", "patch-package": "^6.5.1", "turbo": "^1.10.12" @@ -2604,7 +2606,7 @@ }, "node_modules/@clack/prompts/node_modules/is-unicode-supported": { "version": "1.3.0", - "dev": true, + "extraneous": true, "inBundle": true, "license": "MIT", "engines": { @@ -6287,6 +6289,15 @@ "@types/ms": "*" } }, + "node_modules/@types/dns2": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/@types/dns2/-/dns2-2.0.3.tgz", + "integrity": "sha512-sO14jUYelc2DzwHcCbwp7tZsZfB2x17/zIdHCAeUBINAz2cc36iVFLqCPCB7rn73CzoyoCmpkEnh1rA8C0puPw==", + "dev": true, + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/esprima": { "version": "4.0.3", "resolved": "https://registry.npmjs.org/@types/esprima/-/esprima-4.0.3.tgz", @@ -10709,6 +10720,12 @@ "node": ">=8" } }, + "node_modules/dns2": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/dns2/-/dns2-2.1.0.tgz", + "integrity": "sha512-m27K11aQalRbmUs7RLaz6aPyceLjAoqjPRNTdE7qUouQpl+PC8Bi67O+i9SuJUPbQC8dxFrczAxfmTPuTKHNkw==", + "dev": true + }, "node_modules/doctrine": { "version": "3.0.0", "license": "Apache-2.0", @@ -35548,7 +35565,7 @@ "is-unicode-supported": { "version": "1.3.0", "bundled": true, - "dev": true + "extraneous": true } } }, @@ -38267,6 +38284,15 @@ "@types/ms": "*" } }, + "@types/dns2": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/@types/dns2/-/dns2-2.0.3.tgz", + "integrity": "sha512-sO14jUYelc2DzwHcCbwp7tZsZfB2x17/zIdHCAeUBINAz2cc36iVFLqCPCB7rn73CzoyoCmpkEnh1rA8C0puPw==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/esprima": { "version": "4.0.3", "resolved": "https://registry.npmjs.org/@types/esprima/-/esprima-4.0.3.tgz", @@ -41954,6 +41980,12 @@ "path-type": "^4.0.0" } }, + "dns2": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/dns2/-/dns2-2.1.0.tgz", + "integrity": "sha512-m27K11aQalRbmUs7RLaz6aPyceLjAoqjPRNTdE7qUouQpl+PC8Bi67O+i9SuJUPbQC8dxFrczAxfmTPuTKHNkw==", + "dev": true + }, "doctrine": { "version": "3.0.0", "requires": { diff --git a/package.json b/package.json index 1d24a92efb66..4ccbd7c2a8dc 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,8 @@ }, "devDependencies": { "@cloudflare/workers-types": "^4.20221111.1", + "@types/dns2": "^2.0.3", + "dns2": "^2.1.0", "esbuild": "0.16.3", "patch-package": "^6.5.1", "turbo": "^1.10.12" diff --git a/packages/create-cloudflare/src/helpers/poll.ts b/packages/create-cloudflare/src/helpers/poll.ts index 1acabf281ee1..57bc5a873c65 100644 --- a/packages/create-cloudflare/src/helpers/poll.ts +++ b/packages/create-cloudflare/src/helpers/poll.ts @@ -1,7 +1,8 @@ -import { Resolver } from "node:dns/promises"; +import dns2 from "dns2"; import { request } from "undici"; import { blue, brandColor, dim } from "./colors"; import { spinner } from "./interactive"; +import type { DnsAnswer, DnsResponse } from "types"; const TIMEOUT = 1000 * 60 * 5; const POLL_INTERVAL = 1000; @@ -12,9 +13,11 @@ export const poll = async (url: string): Promise => { const s = spinner(); s.start("Waiting for DNS to propagate"); + await sleep(10 * 1000); + while (Date.now() - start < TIMEOUT) { s.update(`Waiting for DNS to propagate (${secondsSince(start)}s)`); - if (await dnsLookup(domain)) { + if (await isDomainAvailable(domain)) { s.stop(`${brandColor("DNS propagation")} ${dim("complete")}.`); break; } @@ -54,20 +57,43 @@ export const poll = async (url: string): Promise => { return false; }; -async function dnsLookup(domain: string): Promise { +export const isDomainAvailable = async (domain: string) => { try { - const resolver = new Resolver({ timeout: TIMEOUT, tries: 1 }); - resolver.setServers([ - "1.1.1.1", - "1.0.0.1", - "2606:4700:4700::1111", - "2606:4700:4700::1001", - ]); - return (await resolver.resolve4(domain)).length > 0; - } catch (e) { + const nameServers = await lookupAuthoritativeServers(domain); + if (nameServers.length === 0) return false; + + const dns = new dns2({ nameServers }); + const res = await dns.resolve(domain, "A"); + return res.answers.length > 0; + } catch (error) { return false; } -} +}; + +// Looks up the nameservers that are responsible for this particular domain +export const lookupAuthoritativeServers = async (domain: string) => { + const nameServers = await lookupRootNameservers(domain); + const dns = new dns2({ nameServers }); + const res = (await dns.resolve(domain, "NS")) as DnsResponse; + + return ( + res.authorities + // Filter out non-authoritative authorities (ones that don't have an 'ns' property) + .filter((r) => Boolean(r.ns)) + // Return only the hostnames of the authoritative servers + .map((r) => r.ns) + ); +}; + +// Looks up the nameservers responsible for handling `pages.dev` domains +export const lookupRootNameservers = async (domain: string) => { + // `pages.dev` or `workers.dev` + const baseDomain = domain.split(".").slice(-2).join("."); + + const dns = new dns2({}); + const nameservers = await dns.resolve(baseDomain, "NS"); + return (nameservers.answers as DnsAnswer[]).map((n) => n.ns); +}; async function sleep(ms: number) { return new Promise((res) => setTimeout(res, ms)); diff --git a/packages/create-cloudflare/src/types.ts b/packages/create-cloudflare/src/types.ts index ac4e5aaadafa..19773138c2ca 100644 --- a/packages/create-cloudflare/src/types.ts +++ b/packages/create-cloudflare/src/types.ts @@ -1,3 +1,7 @@ +import type { + DnsAnswer as _DnsAnswer, + DnsResponse as _DnsResponse, +} from "dns2"; import type { FrameworkMap } from "frameworks/index"; export type FrameworkName = keyof typeof FrameworkMap; @@ -48,3 +52,12 @@ export type FrameworkConfig = { testFlags?: string[]; compatibilityFlags?: string[]; }; + +// Augmenting the type from the dns2 library since the types are outdated +export interface DnsAnswer extends _DnsAnswer { + ns: string; +} + +export interface DnsResponse extends _DnsResponse { + authorities: DnsAnswer[]; +} From 5176dd3d07125ea95e32c9a6fe8bc3cacaf90a77 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Aug 2023 17:17:01 -0500 Subject: [PATCH 2/4] changeset --- .changeset/hip-lobsters-sniff.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hip-lobsters-sniff.md diff --git a/.changeset/hip-lobsters-sniff.md b/.changeset/hip-lobsters-sniff.md new file mode 100644 index 000000000000..79c977a647fc --- /dev/null +++ b/.changeset/hip-lobsters-sniff.md @@ -0,0 +1,5 @@ +--- +"create-cloudflare": patch +--- + +Improve experience for WARP users From 6cf281b0494b152126f0c2ac587058973e2bffc9 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 10 Aug 2023 11:10:17 -0500 Subject: [PATCH 3/4] Responding to PR feedback --- .changeset/hip-lobsters-sniff.md | 2 +- package-lock.json | 6 +- package.json | 2 - packages/create-cloudflare/dns2.d.ts | 11 ++++ packages/create-cloudflare/package.json | 2 + .../create-cloudflare/src/helpers/poll.ts | 66 ++++++++++++++----- packages/create-cloudflare/src/types.ts | 13 ---- 7 files changed, 67 insertions(+), 35 deletions(-) create mode 100644 packages/create-cloudflare/dns2.d.ts diff --git a/.changeset/hip-lobsters-sniff.md b/.changeset/hip-lobsters-sniff.md index 79c977a647fc..4742f59fb48f 100644 --- a/.changeset/hip-lobsters-sniff.md +++ b/.changeset/hip-lobsters-sniff.md @@ -2,4 +2,4 @@ "create-cloudflare": patch --- -Improve experience for WARP users +Improve experience for WARP users by improving the reliability of the polling logic that waits for newly created apps to become available. diff --git a/package-lock.json b/package-lock.json index 9bf4ba9a6968..997ea97d670d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,8 +35,6 @@ }, "devDependencies": { "@cloudflare/workers-types": "^4.20221111.1", - "@types/dns2": "^2.0.3", - "dns2": "^2.1.0", "esbuild": "0.16.3", "patch-package": "^6.5.1", "turbo": "^1.10.12" @@ -30343,6 +30341,7 @@ "@cloudflare/workers-types": "^4.20230419.0", "@types/command-exists": "^1.2.0", "@types/cross-spawn": "^6.0.2", + "@types/dns2": "^2.0.3", "@types/esprima": "^4.0.3", "@types/node": "^18.15.3", "@types/which-pm-runs": "^1.0.0", @@ -30352,6 +30351,7 @@ "chalk": "^5.2.0", "command-exists": "^1.2.9", "cross-spawn": "^7.0.3", + "dns2": "^2.1.0", "esbuild": "^0.17.12", "execa": "^7.1.1", "haikunator": "^2.1.2", @@ -41018,6 +41018,7 @@ "@cloudflare/workers-types": "^4.20230419.0", "@types/command-exists": "^1.2.0", "@types/cross-spawn": "^6.0.2", + "@types/dns2": "^2.0.3", "@types/esprima": "^4.0.3", "@types/node": "^18.15.3", "@types/which-pm-runs": "^1.0.0", @@ -41027,6 +41028,7 @@ "chalk": "^5.2.0", "command-exists": "^1.2.9", "cross-spawn": "^7.0.3", + "dns2": "^2.1.0", "esbuild": "^0.17.12", "execa": "^7.1.1", "haikunator": "^2.1.2", diff --git a/package.json b/package.json index 4ccbd7c2a8dc..1d24a92efb66 100644 --- a/package.json +++ b/package.json @@ -50,8 +50,6 @@ }, "devDependencies": { "@cloudflare/workers-types": "^4.20221111.1", - "@types/dns2": "^2.0.3", - "dns2": "^2.1.0", "esbuild": "0.16.3", "patch-package": "^6.5.1", "turbo": "^1.10.12" diff --git a/packages/create-cloudflare/dns2.d.ts b/packages/create-cloudflare/dns2.d.ts new file mode 100644 index 000000000000..ddcda4226806 --- /dev/null +++ b/packages/create-cloudflare/dns2.d.ts @@ -0,0 +1,11 @@ +import type {DnsAnswer as _DnsAnswer, DnsResponse as _DnsResponse} from 'dns2' + +declare module 'dns2' { + export interface DnsAnswer extends _DnsAnswer { + ns: string; + } + + export interface DnsResponse extends _DnsResponse { + authorities: DnsAnswer[]; + } +} \ No newline at end of file diff --git a/packages/create-cloudflare/package.json b/packages/create-cloudflare/package.json index b7c5cb1e4de9..926f919a1f51 100644 --- a/packages/create-cloudflare/package.json +++ b/packages/create-cloudflare/package.json @@ -44,6 +44,7 @@ "@cloudflare/workers-types": "^4.20230419.0", "@types/command-exists": "^1.2.0", "@types/cross-spawn": "^6.0.2", + "@types/dns2": "^2.0.3", "@types/esprima": "^4.0.3", "@types/node": "^18.15.3", "@types/which-pm-runs": "^1.0.0", @@ -53,6 +54,7 @@ "chalk": "^5.2.0", "command-exists": "^1.2.9", "cross-spawn": "^7.0.3", + "dns2": "^2.1.0", "esbuild": "^0.17.12", "execa": "^7.1.1", "haikunator": "^2.1.2", diff --git a/packages/create-cloudflare/src/helpers/poll.ts b/packages/create-cloudflare/src/helpers/poll.ts index 57bc5a873c65..54fdbae5acdb 100644 --- a/packages/create-cloudflare/src/helpers/poll.ts +++ b/packages/create-cloudflare/src/helpers/poll.ts @@ -2,28 +2,62 @@ import dns2 from "dns2"; import { request } from "undici"; import { blue, brandColor, dim } from "./colors"; import { spinner } from "./interactive"; -import type { DnsAnswer, DnsResponse } from "types"; +import type { DnsAnswer, DnsResponse } from "dns2"; const TIMEOUT = 1000 * 60 * 5; const POLL_INTERVAL = 1000; +/* + A helper to wait until the newly deployed domain is available. + + We do this by first polling DNS until the new domain is resolvable, and then polling + via HTTP until we get a successful response. + + Note that when polling DNS we make queries against specific nameservers to avoid negative + caching. Similarly, we poll via HTTP using the 'no-cache' header for the same reason. +*/ export const poll = async (url: string): Promise => { const start = Date.now(); const domain = new URL(url).host; const s = spinner(); s.start("Waiting for DNS to propagate"); + + // Start out by sleeping for 10 seconds since it's unlikely DNS changes will + // have propogated before then await sleep(10 * 1000); + await pollDns(domain, start, s); + if (await pollHttp(url, start, s)) return true; + + s.stop( + `${brandColor( + "timed out" + )} while waiting for ${url} - try accessing it in a few minutes.` + ); + return false; +}; + +const pollDns = async ( + domain: string, + start: number, + s: ReturnType +) => { while (Date.now() - start < TIMEOUT) { s.update(`Waiting for DNS to propagate (${secondsSince(start)}s)`); - if (await isDomainAvailable(domain)) { + if (await isDomainResolvable(domain)) { s.stop(`${brandColor("DNS propagation")} ${dim("complete")}.`); - break; + return; } await sleep(POLL_INTERVAL); } +}; +const pollHttp = async ( + url: string, + start: number, + s: ReturnType +) => { s.start("Waiting for deployment to become available"); while (Date.now() - start < TIMEOUT) { s.update( @@ -48,20 +82,18 @@ export const poll = async (url: string): Promise => { } await sleep(POLL_INTERVAL); } - - s.stop( - `${brandColor( - "timed out" - )} while waiting for ${url} - try accessing it in a few minutes.` - ); - return false; }; -export const isDomainAvailable = async (domain: string) => { +// Determines if the domain is resolvable via DNS. Until this condition is true, +// any HTTP requests will result in an NXDOMAIN error. +export const isDomainResolvable = async (domain: string) => { try { - const nameServers = await lookupAuthoritativeServers(domain); + const nameServers = await lookupSubdomainNameservers(domain); + + // If the subdomain nameservers aren't resolvable yet, keep polling if (nameServers.length === 0) return false; + // Once they are resolvable, query these nameservers for the domain's 'A' record const dns = new dns2({ nameServers }); const res = await dns.resolve(domain, "A"); return res.answers.length > 0; @@ -71,8 +103,8 @@ export const isDomainAvailable = async (domain: string) => { }; // Looks up the nameservers that are responsible for this particular domain -export const lookupAuthoritativeServers = async (domain: string) => { - const nameServers = await lookupRootNameservers(domain); +export const lookupSubdomainNameservers = async (domain: string) => { + const nameServers = await lookupDomainLevelNameservers(domain); const dns = new dns2({ nameServers }); const res = (await dns.resolve(domain, "NS")) as DnsResponse; @@ -85,9 +117,9 @@ export const lookupAuthoritativeServers = async (domain: string) => { ); }; -// Looks up the nameservers responsible for handling `pages.dev` domains -export const lookupRootNameservers = async (domain: string) => { - // `pages.dev` or `workers.dev` +// Looks up the nameservers responsible for handling `pages.dev` or `workers.dev` domains +export const lookupDomainLevelNameservers = async (domain: string) => { + // Get the last 2 parts of the domain (ie. `pages.dev` or `workers.dev`) const baseDomain = domain.split(".").slice(-2).join("."); const dns = new dns2({}); diff --git a/packages/create-cloudflare/src/types.ts b/packages/create-cloudflare/src/types.ts index 19773138c2ca..ac4e5aaadafa 100644 --- a/packages/create-cloudflare/src/types.ts +++ b/packages/create-cloudflare/src/types.ts @@ -1,7 +1,3 @@ -import type { - DnsAnswer as _DnsAnswer, - DnsResponse as _DnsResponse, -} from "dns2"; import type { FrameworkMap } from "frameworks/index"; export type FrameworkName = keyof typeof FrameworkMap; @@ -52,12 +48,3 @@ export type FrameworkConfig = { testFlags?: string[]; compatibilityFlags?: string[]; }; - -// Augmenting the type from the dns2 library since the types are outdated -export interface DnsAnswer extends _DnsAnswer { - ns: string; -} - -export interface DnsResponse extends _DnsResponse { - authorities: DnsAnswer[]; -} From 2ea5f120171ce1e88637add6726bfc6b69e9ec68 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 10 Aug 2023 14:24:03 -0500 Subject: [PATCH 4/4] Removing project cleanup after e2e runs -- failing and will be fixed in another pr --- .../create-cloudflare/e2e-tests/pages.test.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/packages/create-cloudflare/e2e-tests/pages.test.ts b/packages/create-cloudflare/e2e-tests/pages.test.ts index a2f937592cc9..bf5d9c33c08f 100644 --- a/packages/create-cloudflare/e2e-tests/pages.test.ts +++ b/packages/create-cloudflare/e2e-tests/pages.test.ts @@ -2,7 +2,6 @@ import { existsSync, mkdtempSync, realpathSync, rmSync } from "fs"; import crypto from "node:crypto"; import { tmpdir } from "os"; import { join } from "path"; -import spawn from "cross-spawn"; import { FrameworkMap } from "frameworks/index"; import { readJSON } from "helpers/files"; import { fetch } from "undici"; @@ -39,29 +38,10 @@ describe(`E2E: Web frameworks`, () => { afterEach((ctx) => { const framework = ctx.meta.name; const projectPath = getProjectPath(framework); - const projectName = getProjectName(framework); if (existsSync(projectPath)) { rmSync(projectPath, { recursive: true }); } - - try { - const { output } = spawn.sync("npx", [ - "wrangler", - "pages", - "project", - "delete", - "-y", - projectName, - ]); - - if (!output.toString().includes(`Successfully deleted ${projectName}`)) { - console.error(output.toString()); - } - } catch (error) { - console.error(`Failed to cleanup project: ${projectName}`); - console.error(error); - } }); const runCli = async (