fix: image optimizer hangs when invalid image is requested by kyliau · Pull Request #33719 · vercel/next.js

kyliau · 2022-01-27T00:37:22Z

When an invalid image is requested, the 'finish' event is never triggered,
which ultimately leads to a 504 Gateway Timeout error.

This is fixed by invoking the 'callback' in _write().

fix #33441

Bug

Related issues linked using fixes #number
Integration tests added
Errors have helpful link attached, see 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
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.
Errors have helpful link attached, see contributing.md

Documentation / Examples

Make sure the linting passes by running yarn lint

styfle · 2022-01-27T00:46:24Z

Suggested change

callback()

if (callback) callback()

The docs mention that encoding and callback are both optional parameters so best to check it exists

I think the two parameters are optional when calling write()
https://nodejs.org/docs/latest-v16.x/api/stream.html#writablewritechunk-encoding-callback
but not when calling _write()
https://nodejs.org/docs/latest-v16.x/api/stream.html#writable_writechunk-encoding-callback

When an invalid image is requested, the 'finish' event is never triggered, which ultimately leads to a 504 Gateway Timeout error. This is fixed by invoking the 'callback' in `_write()`. fix vercel#33441

styfle

Great work, thanks! 🎉

kyliau · 2022-01-27T01:07:05Z

also, FWIW, the following test should have failed on CI but apparently it does not.
On my local machine the test hangs.

next.js/test/integration/image-optimizer/test/index.test.js

Lines 774 to 780 in eaf680d

    
           it("should error if the resource isn't a valid image", async () => { 
        
             const query = { url: '/test.txt', w, q: 80 } 
        
             const opts = { headers: { accept: 'image/webp' } } 
        
             const res = await fetchViaHTTP(appPort, '/_next/image', query, opts) 
        
             expect(res.status).toBe(400) 
        
             expect(await res.text()).toBe("The requested resource isn't a valid image.") 
        
           })

ijjk · 2022-01-27T01:23:25Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
buildDuration	12.5s	12.7s	⚠️ +174ms
buildDurationCached	3.4s	3.3s	-27ms
nodeModulesSize	356 MB	356 MB	⚠️ +589 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.912	2.94	⚠️ +0.03
/ avg req/sec	858.39	850.36	⚠️ -8.03
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.381	1.367	-0.01
/error-in-render avg req/sec	1810.45	1829.21	+18.76

Client Bundles (main, webpack, commons)

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	27.2 kB	27.2 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71 kB	71 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
_app-HASH.js gzip	1.37 kB	1.37 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	326 B	326 B	✓
dynamic-HASH.js gzip	2.37 kB	2.37 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	919 B	919 B	✓
image-HASH.js gzip	4.94 kB	4.94 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.19 kB	2.19 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.4 kB	14.4 kB	✓

Client Build Manifests

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Default Build with SWC (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
buildDuration	16s	16s	⚠️ +19ms
buildDurationCached	3.4s	3.3s	-44ms
nodeModulesSize	356 MB	356 MB	⚠️ +589 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.944	2.947	0
/ avg req/sec	849.1	848.33	⚠️ -0.77
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.334	1.351	⚠️ +0.02
/error-in-render avg req/sec	1874.38	1850.98	⚠️ -23.4

Client Bundles (main, webpack, commons)

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	27.3 kB	27.3 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.3 kB	71.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.36 kB	2.36 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	911 B	911 B	✓
image-HASH.js gzip	4.97 kB	4.97 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.21 kB	2.21 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.3 kB	14.3 kB	✓

Client Build Manifests

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	kyliau/next.js image-optimizer	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: a5cd3f5

kyliau requested review from ijjk, shuding, styfle and timneutkens as code owners January 27, 2022 00:37

ijjk added created-by: Chrome Aurora type: next labels Jan 27, 2022

styfle reviewed Jan 27, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

fix: image optimizer hangs when invalid image is requested

08e09a5

When an invalid image is requested, the 'finish' event is never triggered, which ultimately leads to a 504 Gateway Timeout error. This is fixed by invoking the 'callback' in `_write()`. fix vercel#33441

kyliau force-pushed the image-optimizer branch from 8ec6375 to 08e09a5 Compare January 27, 2022 01:00

styfle approved these changes Jan 27, 2022

View reviewed changes

Merge branch 'canary' into image-optimizer

a5cd3f5

kodiakhq Bot merged commit c551a32 into vercel:canary Jan 27, 2022

kyliau deleted the image-optimizer branch January 27, 2022 05:49

bonesoul mentioned this pull request Feb 1, 2022

"ENOENT: no such file or directory" - next.js crashes, related to image caching? #33860

Closed

freender mentioned this pull request Feb 14, 2022

Overseerr crashes when invalid image is requested sct/overseerr#2549

Closed

github-actions Bot locked as resolved and limited conversation to collaborators Feb 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: image optimizer hangs when invalid image is requested#33719

fix: image optimizer hangs when invalid image is requested#33719
kodiakhq[bot] merged 2 commits intovercel:canaryfrom
kyliau:image-optimizer

kyliau commented Jan 27, 2022

Uh oh!

styfle Jan 27, 2022

Uh oh!

kyliau Jan 27, 2022

Uh oh!

This comment has been minimized.

styfle left a comment

Uh oh!

kyliau commented Jan 27, 2022

Uh oh!

ijjk commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kyliau commented Jan 27, 2022

Bug

Feature

Documentation / Examples

Uh oh!

styfle Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

kyliau Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

styfle left a comment

Choose a reason for hiding this comment

Uh oh!

kyliau commented Jan 27, 2022

Uh oh!

ijjk commented Jan 27, 2022

Stats from current PR

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants