Add retry for fetching subsetted Google Fonts #56583

berlysia · 2023-10-08T10:38:11Z

What?

Add the same re-retrieval process for subseted font files of Google Font as for CSS files.

make use of async-retry

Why?

It was reported in #45080 that Japanese fonts such as Noto Sans JP were frequently Failed to fetch.
A retry process was added in #51890, but it did not resolve the issue completely ( #51890 (comment) ).

Here is my reproduction code with 13.5.5-canary.4 (please run locally).
https://stackblitz.com/edit/stackblitz-starters-n8zxlq?file=app%2Fpage.tsx

And my local error log is here(folded)

$ npm run -- dev

> nextjs@0.1.0 dev
> next dev

 ⚠ Port 3000 is in use, trying 3001 instead.
  ▲ Next.js 13.5.5-canary.4
  - Local:        http://localhost:3001

 ✓ Ready in 23.9s
 ○ Compiling /page ...
FetchError: request to https://fonts.gstatic.com/s/notosansjp/v52/-F6jfjtqLzI2JPCgQBnw7HFyzSD-AsregP8VFBEj757Y0rw_qMHVdbR2L8Y9QTJ1LwkRmR5GprQAe69m.4.woff2 failed, reason: 
    at ClientRequest.<anonymous> (/mnt/c/Users/berlysia/Downloads/stackblitz-starters-n8zxlq/node_modules/next/dist/compiled/node-fetch/index.js:1:65756)
    at ClientRequest.emit (node:events:514:28)
    at TLSSocket.socketErrorListener (node:_http_client:495:9)
    at TLSSocket.emit (node:events:514:28)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  type: 'system',
  errno: 'ETIMEDOUT',
  code: 'ETIMEDOUT'
}
 ⨯ Failed to download `Noto Sans JP` from Google Fonts. Using fallback font instead.

Failed to fetch `Noto Sans JP` from Google Fonts.}
FetchError: request to https://fonts.gstatic.com/s/notosansjp/v52/-F6jfjtqLzI2JPCgQBnw7HFyzSD-AsregP8VFBEj757Y0rw_qMHVdbR2L8Y9QTJ1LwkRmR5GprQAe69m.28.woff2 failed, reason: 
    at ClientRequest.<anonymous> (/mnt/c/Users/berlysia/Downloads/stackblitz-starters-n8zxlq/node_modules/next/dist/compiled/node-fetch/index.js:1:65756)
    at ClientRequest.emit (node:events:514:28)
    at TLSSocket.socketErrorListener (node:_http_client:495:9)
    at TLSSocket.emit (node:events:514:28)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at processTicksAndRejections (node:internal/process/task_queues:82:21)
    at runNextTicks (node:internal/process/task_queues:64:3)
    at listOnTimeout (node:internal/timers:540:9)
    at process.processTimers (node:internal/timers:514:7) {
  type: 'system',
  errno: 'ETIMEDOUT',
  code: 'ETIMEDOUT'
}
...(15 errors emitted)

I've found that the issue is not limited to fetching CSS, fetching subset font files is also failing.
By adding retry handling to the fetch of individual subseted font files as well, I (almost) never see Failed to fetch anymore.

The issue tends to become more apparent when downloading a larger number of subsetted fonts.
This suggests that the problem is more likely to occur with larger fonts, such as those designed for CJK languages.

How?

Add the same re-retrieval process for subseted font files of Google Font as for CSS files.

Related to #51890 #53239 #45080 #53279

SukkaW · 2023-10-09T05:03:04Z

packages/font/src/google/fetch-font-file.ts

@@ -1,6 +1,7 @@
 // @ts-ignore
 import fetch from 'next/dist/compiled/node-fetch'
 import { getProxyAgent } from './get-proxy-agent'
+import { retry } from './retry'


Let's use async-retry instead of the self-baked solution.

berlysia · 2023-10-09T09:28:45Z

packages/font/package.json

    "@types/fontkit": "2.0.0",
    "@vercel/ncc": "0.34.0",
+    "async-retry": "1.3.3",


NOTE: async-retry is already in devDeps at root and create-next-app

🔗 #56583 (comment)

berlysia · 2023-10-09T09:34:11Z

packages/font/src/google/fetch-css-from-google-fonts.ts

@@ -1,24 +1,10 @@
 // @ts-ignore
 import fetch from 'next/dist/compiled/node-fetch'
+// eslint-disable-next-line import/no-extraneous-dependencies


import/no-extraneous-dependencies is now useless because of ncc, right?

introduced by #13482
disable directives: https://github.com/search?q=repo%3Avercel%2Fnext.js+%22import%2Fno-extraneous-dependencies%22+%22eslint-disable%22&type=code

current configuration:

next.js/.eslintrc.json

Lines 163 to 176 in 9b2f29e

{

"files": ["packages/**"],

"excludedFiles": [

"packages/next/taskfile*.js",

"packages/next/webpack.config.js"

],

"rules": {

"no-shadow": ["warn", { "builtinGlobals": false }],

"import/no-extraneous-dependencies": [

"error",

{ "devDependencies": false }

]

}

},

berlysia · 2023-10-09T09:51:38Z

packages/font/src/google/retry.ts

+        (e as Error).message + `\n\nRetrying ${attempt}/${retries}...`
+      )
+    },
+    minTimeout: 100,


NOTE: The cooldown time for retries is based on the original code.
However, the second retry and later, the wait time doubles with each subsequent retry. This behavior is due to the default settings of async-retry and can be modified if needed. async-retry API

That's the default "Exponential Backoff" behavior of async-retry and IMHO we should keep it.

ijjk · 2023-10-13T19:52:11Z

Allow CI Workflow Run

approve CI run for commit: b422eaa

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ijjk · 2023-10-13T19:52:11Z

Allow CI Workflow Run

approve CI run for commit: b422eaa

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ijjk · 2023-10-13T19:52:12Z

Allow CI Workflow Run

approve CI run for commit: b422eaa

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ijjk · 2023-10-13T20:11:29Z

Stats from current PR

Default Build

General Overall increase ⚠️

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
buildDuration	10.6s	10.5s	N/A
buildDurationCached	6.1s	6.1s	N/A
nodeModulesSize	174 MB	174 MB	⚠️ +1.16 kB
nextStartRea..uration (ms)	515ms	497ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
199-HASH.js gzip	27.5 kB	27.5 kB	✓
3f784ff6-HASH.js gzip	53.3 kB	53.3 kB	✓
99.HASH.js gzip	182 B	182 B	✓
framework-HASH.js gzip	45.3 kB	45.3 kB	✓
main-app-HASH.js gzip	254 B	251 B	N/A
main-HASH.js gzip	32.9 kB	32.9 kB	N/A
webpack-HASH.js gzip	1.75 kB	1.75 kB	N/A
Overall change	126 kB	126 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
_app-HASH.js gzip	206 B	205 B	N/A
_error-HASH.js gzip	182 B	180 B	N/A
amp-HASH.js gzip	506 B	505 B	N/A
css-HASH.js gzip	322 B	323 B	N/A
dynamic-HASH.js gzip	2.57 kB	2.57 kB	N/A
edge-ssr-HASH.js gzip	260 B	259 B	N/A
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	369 B	369 B	✓
image-HASH.js gzip	4.36 kB	4.36 kB	N/A
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.64 kB	2.63 kB	N/A
routerDirect..HASH.js gzip	312 B	311 B	N/A
script-HASH.js gzip	385 B	384 B	N/A
withRouter-HASH.js gzip	307 B	308 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	1.08 kB	1.08 kB	✓

Client Build Manifests

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
_buildManifest.js gzip	484 B	482 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
index.html gzip	528 B	529 B	N/A
link.html gzip	543 B	542 B	N/A
withRouter.html gzip	524 B	524 B	✓
Overall change	524 B	524 B	✓

Edge SSR bundle Size

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
edge-ssr.js gzip	94 kB	94 kB	N/A
page.js gzip	155 kB	155 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	berlysia/next.js add-retry-for-font-fetching	Change
middleware-b..fest.js gzip	624 B	624 B	✓
middleware-r..fest.js gzip	150 B	151 B	N/A
middleware.js gzip	22.9 kB	22.9 kB	N/A
edge-runtime..pack.js gzip	1.92 kB	1.92 kB	✓
Overall change	2.55 kB	2.55 kB	✓

Commit: 9cde18b

smorimoto · 2023-10-16T15:49:54Z

This PR picks up where I missed it and I think we should merge this and do a point release soon as this addresses a pretty serious issue.
We have not seen instability on the Vercel builder for months, but in the last few days it has become unstable even on the Vercel builder, and manually clicking redeploy over and over again is a waste of time.
We build Next.js an incredible number of times a day all over the world, so I'd like to see the Vercel team do better. This addresses things, but doesn't fix anything.

styfle

Looks good, thanks! 🎉

smorimoto · 2023-10-20T19:02:58Z

@styfle Thanks for taking care of this!
Tell Gabe I was so grateful 😄

This reverts commit c84f5ed.

styfle · 2023-10-20T21:09:59Z

Reverting due to an error #57154

Reverts #56583 due to missing dependency ``` app/layout.jsx An error occured in 'next/font'. Error: Cannot find module 'async-retry' Require stack: - node_modules/•pnpm/next@13.5.7-canary.12_react-dom@18.2.0_react@18.2.0/node_modules/next/dist/compiled/@next/font/dist/google/retry.js - node_modules/-pnpm/next@13.5.7-canary-12_react-dom@18.2.0_react@18.2.0/node_modules/next/dist/compiled/@next/font/dist/google/fetch-css-from-google-fonts.js ```

berlysia · 2023-10-23T17:41:39Z

Hello @styfle ,

I noticed that this PR was reverted. I'm confident that my changes still can resolve the issue.

However, I'm not certain why the Cannot find module error is occurring. I've suggested two approaches to resolve this error:

Add async-retry to the dependencies. Use async-retry for font fetching as runtime dependency #57276
Replace async-retry with next/dist/compiled/async-retry in the import statement. Use compiled async-retry for font fetching #57275
- It works on stackblitz

Which approach do you prefer, or another?

I'm not familiar with the behavior of the ncc library or the way of next.js , so I can't say with confidence which solution is best at this moment.

berlysia requested review from timneutkens, ijjk, shuding, huozhi, feedthejim, ztanner and wyattjoh as code owners October 8, 2023 10:38

ijjk added Font (next/font) Related to Next.js Font Optimization. type: next labels Oct 8, 2023

SukkaW reviewed Oct 9, 2023

View reviewed changes

berlysia force-pushed the add-retry-for-font-fetching branch from bf906a4 to b3ceafc Compare October 9, 2023 09:25

berlysia requested review from a team as code owners October 9, 2023 09:25

berlysia commented Oct 9, 2023

View reviewed changes

berlysia force-pushed the add-retry-for-font-fetching branch from b3ceafc to db57027 Compare October 9, 2023 09:44

berlysia commented Oct 9, 2023

View reviewed changes

berlysia force-pushed the add-retry-for-font-fetching branch from db57027 to b422eaa Compare October 11, 2023 21:49

berlysia changed the title ~~Apply retry utility to fetching Google Font subsetted files~~ Add retry for fetching subsetted Google Fonts Oct 11, 2023

smorimoto mentioned this pull request Oct 16, 2023

Add more retries to font fetching #53239

Closed

berlysia added 3 commits October 17, 2023 03:24

extract retry function

db77ccc

make use of async-retry

502c20e

add retry for font fetching

4e3d0a4

berlysia force-pushed the add-retry-for-font-fetching branch from b422eaa to 4e3d0a4 Compare October 16, 2023 18:24

styfle added the CI approved Approve running CI for fork label Oct 20, 2023

styfle added 2 commits October 20, 2023 10:25

Merge branch 'canary' into add-retry-for-font-fetching

9fcf25a

Merge branch 'canary' into add-retry-for-font-fetching

9cde18b

styfle approved these changes Oct 20, 2023

View reviewed changes

kodiakhq bot merged commit c84f5ed into vercel:canary Oct 20, 2023
53 of 58 checks passed

berlysia deleted the add-retry-for-font-fetching branch October 20, 2023 18:58

styfle added a commit that referenced this pull request Oct 20, 2023

Revert "Add retry for fetching subsetted Google Fonts (#56583)"

35a6c1d

This reverts commit c84f5ed.

styfle mentioned this pull request Oct 20, 2023

Revert "Add retry for fetching subsetted Google Fonts" #57154

Merged

This was referenced Oct 23, 2023

Use compiled async-retry for font fetching #57275

Closed

Use async-retry for font fetching as runtime dependency #57276

Closed

github-actions bot added the locked label Nov 7, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add retry for fetching subsetted Google Fonts #56583

Add retry for fetching subsetted Google Fonts #56583

berlysia commented Oct 8, 2023 •

edited

Loading

SukkaW Oct 9, 2023

berlysia Oct 9, 2023

berlysia Oct 9, 2023

berlysia Oct 9, 2023

SukkaW Oct 9, 2023 •

edited

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited

Loading

smorimoto commented Oct 16, 2023

styfle left a comment

smorimoto commented Oct 20, 2023

styfle commented Oct 20, 2023

berlysia commented Oct 23, 2023

	{
	"files": ["packages/**"],
	"excludedFiles": [
	"packages/next/taskfile*.js",
	"packages/next/webpack.config.js"
	],
	"rules": {
	"no-shadow": ["warn", { "builtinGlobals": false }],
	"import/no-extraneous-dependencies": [
	"error",
	{ "devDependencies": false }
	]
	}
	},

Add retry for fetching subsetted Google Fonts #56583

Add retry for fetching subsetted Google Fonts #56583

Conversation

berlysia commented Oct 8, 2023 • edited Loading

What?

Why?

How?

SukkaW Oct 9, 2023

Choose a reason for hiding this comment

berlysia Oct 9, 2023

Choose a reason for hiding this comment

berlysia Oct 9, 2023

Choose a reason for hiding this comment

berlysia Oct 9, 2023

Choose a reason for hiding this comment

SukkaW Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

ijjk commented Oct 13, 2023 • edited by styfle Loading

ijjk commented Oct 13, 2023 • edited by styfle Loading

ijjk commented Oct 13, 2023 • edited by styfle Loading

ijjk commented Oct 13, 2023 • edited Loading

Stats from current PR

smorimoto commented Oct 16, 2023

styfle left a comment

Choose a reason for hiding this comment

smorimoto commented Oct 20, 2023

styfle commented Oct 20, 2023

berlysia commented Oct 23, 2023

berlysia commented Oct 8, 2023 •

edited

Loading

SukkaW Oct 9, 2023 •

edited

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited by styfle

Loading

ijjk commented Oct 13, 2023 •

edited

Loading