From 14bfe87110fee29cda6dc30c29c0a2e336b069dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Born=C3=B6?= Date: Tue, 7 Mar 2023 13:57:43 +0100 Subject: [PATCH] Add a timeout to next/font/google in dev (#46834) Add a timeout to the Google Fonts fetch calls in dev. In case they aren't fetched in time, the fallback font is used instead. Currently if font fetching fails due to network errors in dev, the loader is not cached. Every change on a page that uses a font makes it retry to refetch the font. But, if the network is slow enough to cause timeouts, that means that every change would force the user to wait for the timeout before the page being updated. Because of this, the PR also enables the loader to be cached on error. The drawback is that you would have to delete the `.next` folder to retry to get the actual font in case it got cached after failing. Closes NEXT-726 ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --- .../font/src/google/fetch-css-from-google-fonts.ts | 9 ++++++++- packages/font/src/google/fetch-font-file.ts | 13 +++++++++++-- packages/font/src/google/loader.ts | 6 ++---- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/font/src/google/fetch-css-from-google-fonts.ts b/packages/font/src/google/fetch-css-from-google-fonts.ts index f825c64964..2730afecd1 100644 --- a/packages/font/src/google/fetch-css-from-google-fonts.ts +++ b/packages/font/src/google/fetch-css-from-google-fonts.ts @@ -11,7 +11,8 @@ import { nextFontError } from '../next-font-error' */ export async function fetchCSSFromGoogleFonts( url: string, - fontFamily: string + fontFamily: string, + isDev: boolean ): Promise { // Check if mocked responses are defined, if so use them instead of fetching from Google Fonts let mockedResponse: string | undefined @@ -28,12 +29,18 @@ export async function fetchCSSFromGoogleFonts( // Just use the mocked CSS if it's set cssResponse = mockedResponse } else { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 3000) const res = await fetch(url, { + // Add a timeout in dev + signal: isDev ? controller.signal : undefined, headers: { // The file format is based off of the user agent, make sure woff2 files are fetched 'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36', }, + }).finally(() => { + clearTimeout(timeoutId) }) if (!res.ok) { diff --git a/packages/font/src/google/fetch-font-file.ts b/packages/font/src/google/fetch-font-file.ts index 4296a34ac2..e21e8c0412 100644 --- a/packages/font/src/google/fetch-font-file.ts +++ b/packages/font/src/google/fetch-font-file.ts @@ -4,7 +4,7 @@ import fetch from 'next/dist/compiled/node-fetch' /** * Fetch the url and return a buffer with the font file. */ -export async function fetchFontFile(url: string) { +export async function fetchFontFile(url: string, isDev: boolean) { // Check if we're using mocked data if (process.env.NEXT_FONT_GOOGLE_MOCKED_RESPONSES) { // If it's an absolute path, read the file from the filesystem @@ -15,6 +15,15 @@ export async function fetchFontFile(url: string) { return Buffer.from(url) } - const arrayBuffer = await fetch(url).then((r: any) => r.arrayBuffer()) + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 3000) + const arrayBuffer = await fetch(url, { + // Add a timeout in dev + signal: isDev ? controller.signal : undefined, + }) + .then((r: any) => r.arrayBuffer()) + .finally(() => { + clearTimeout(timeoutId) + }) return Buffer.from(arrayBuffer) } diff --git a/packages/font/src/google/loader.ts b/packages/font/src/google/loader.ts index ca44b9fec0..bee68caf4a 100644 --- a/packages/font/src/google/loader.ts +++ b/packages/font/src/google/loader.ts @@ -32,7 +32,6 @@ const nextFontGoogleFontLoader: FontLoader = async ({ emitFontFile, isDev, isServer, - loaderContext, }) => { const { fontFamily, @@ -83,7 +82,7 @@ const nextFontGoogleFontLoader: FontLoader = async ({ // Fetch CSS from Google Fonts or get it from the cache let fontFaceDeclarations = hasCachedCSS ? cssCache.get(url) - : await fetchCSSFromGoogleFonts(url, fontFamily).catch(() => null) + : await fetchCSSFromGoogleFonts(url, fontFamily, isDev).catch(() => null) if (!hasCachedCSS) { cssCache.set(url, fontFaceDeclarations ?? null) } else { @@ -109,7 +108,7 @@ const nextFontGoogleFontLoader: FontLoader = async ({ // Download the font file or get it from cache const fontFileBuffer = hasCachedFont ? fontCache.get(googleFontFileUrl) - : await fetchFontFile(googleFontFileUrl).catch(() => null) + : await fetchFontFile(googleFontFileUrl, isDev).catch(() => null) if (!hasCachedFont) { fontCache.set(googleFontFileUrl, fontFileBuffer ?? null) } else { @@ -157,7 +156,6 @@ const nextFontGoogleFontLoader: FontLoader = async ({ css: updatedCssResponse, } } catch (err) { - loaderContext.cacheable(false) if (isDev) { if (isServer) { console.error(err)