Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing static file causes memory bloat #2685

Closed
2 of 5 tasks
grimasod opened this issue Apr 8, 2019 · 2 comments
Closed
2 of 5 tasks

Missing static file causes memory bloat #2685

grimasod opened this issue Apr 8, 2019 · 2 comments
Labels
bug Bug reports
Milestone

Comments

@grimasod
Copy link
Contributor

grimasod commented Apr 8, 2019

Current behavior

A missing static file can cause the server to bloat and crash.

  • The default request handler app.get('*') is for a Vue request
  • The static file request handler, eg app.use('/dist') allows fallthrough
  • Therefore, when a static (image or JavaScript) file is not found, the request falls through to the default handler
  • The default handler attempts to render the page, which results in an error
  • The error handler is called and it redirects to the "page-not-found" route
  • The Page Not Found request is unnecessarily rendered and returned instead of a standard 404 status

This situation, a simple missing file, causes an increase in memory usage on the server and is worth fixing. However, a more serious condition can arise.

  • If a theme file is missing, such as a header logo, the situation described above occurs
  • When the Page Not Found is being rendered, the missing file is requested again
  • The same steps are repeated and there's another redirect to page-not-found
  • The server attempts to render the Page Not Found request again
  • This is an infinite loop. Each image request results in a further page-not-found redirect and another request for the missing image and so on.

As this happens, each request uses more memory and very soon the limit is reached. In production mode, PM2 should then kill the server and restart it, but the problem remains.

Expected behavior

A 404 file-not-found status is returned

Steps to reproduce the issue

  1. Delete /assets/logo.svg from a default installation
  2. Open any page in a browser
  3. Observe memory usage on the server

Repository

Can you handle fixing this bug by yourself?

  • YES
  • NO

Simple fix. Preparing a Pull Request

Which Release Cycle state this refers to? Info for developer.

Pick one option.

  • This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

  • Browser: Chrome
  • OS: Mac
  • Node: 8.11.4
  • Code Version: 1.8.4, master, develop

Additional information

@grimasod grimasod added the bug Bug reports label Apr 8, 2019
@patzick patzick added this to the 1.10.0-rc.1 milestone Apr 8, 2019
@patzick
Copy link
Collaborator

patzick commented Apr 8, 2019

Thanks @grimasod !

@patzick patzick closed this as completed Apr 8, 2019
@pkarw
Copy link
Collaborator

pkarw commented Apr 9, 2019

Awesome fix @grimasod - great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants