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.

Sign up for GitHub

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

Jump to bottom

Cover Image slows down requests #8041

Closed
sebgie opened this issue Feb 23, 2017 · 5 comments · Fixed by #8283
Closed

Cover Image slows down requests #8041

sebgie opened this issue Feb 23, 2017 · 5 comments · Fixed by #8283
Assignees
@kirrg001
Labels
bug [triage] something behaving unexpectedly server / core Issues relating to the server or core of Ghost
Milestone
1.0.0 Beta Ready

Comments

@sebgie
Copy link
Contributor

sebgie commented Feb 23, 2017

Issue Summary

When an image from an external source is included as cover image for a post the image size is calculated when the post is first loaded. This works quite well under normal circumstances. If the external image is loading slowly or not at all the request is stuck while the image is loading.

Steps to Reproduce

  1. Create a new post with a cover image that is loading slowly
  2. Publish
  3. Visit the post
  4. Observe the first request being slowed down by loading the image while rendering

Short term fix would be to set the already existent timeout to an acceptable value (https://github.com/TryGhost/Ghost/blob/master/core/server/utils/image-size-from-url.js#L33).

Long term we should store the image size at the time when the image is added.

Technical details:

  • Ghost Version: master & 0.11.4
  • Node Version: LTS 4
  • Browser/OS: Chrome
  • Database: SQLite & MySQL
@aileen aileen mentioned this issue Feb 23, 2017
Merged
aileen added a commit to aileen/Ghost that referenced this issue Mar 1, 2017
@aileen
⏰ Call getImageSize with timeout
ec78842 
refs TryGhost#8041

Calls `getImageSize` with an timeout of 6sec. and adds a default timeout (in case, function is called without optional timeout) of 10sec, to prevent node from using its default timeout of 2minutes 😱
kirrg001 pushed a commit that referenced this issue Mar 1, 2017
@aileen @kirrg001
⏰ Call getImageSize with timeout (#8044)
a109bfb 
refs #8041

Calls `getImageSize` with an timeout of 6sec. and adds a default timeout (in case, function is called without optional timeout) of 10sec, to prevent node from using its default timeout of 2minutes 😱
@kirrg001
Copy link
Contributor

kirrg001 commented Mar 1, 2017

@AileenCGN Could you PR the fix to master as well?Thanks :)

@ErisDS
Copy link
Member

ErisDS commented Mar 20, 2017

@kirrg001 can you please copy over this change before the next release?

@ErisDS ErisDS assigned kirrg001 and unassigned aileen Mar 20, 2017
@kirrg001
Copy link
Contributor

👍

kirrg001 pushed a commit to kirrg001/Ghost that referenced this issue Mar 20, 2017
@aileen @kirrg001
⏰ Call getImageSize with timeout (TryGhost#8044)
c3af463 
refs TryGhost#8041

Calls `getImageSize` with an timeout of 6sec. and adds a default timeout (in case, function is called without optional timeout) of 10sec, to prevent node from using its default timeout of 2minutes 😱
@kirrg001 kirrg001 mentioned this issue Mar 20, 2017
Merged
ErisDS pushed a commit that referenced this issue Mar 20, 2017
@kirrg001 @ErisDS
⏰ Call getImageSize with timeout (#8044) (#8189)
4e3e1bd 
refs #8041

Calls `getImageSize` with an timeout of 6sec. and adds a default timeout (in case, function is called without optional timeout) of 10sec, to prevent node from using its default timeout of 2minutes 😱
@ErisDS
Copy link
Member

ErisDS commented Mar 20, 2017

This was closed by #8044 and #8189. We can revisit a longer term fix later if we need to.

@ErisDS ErisDS closed this as completed Mar 20, 2017
@ErisDS ErisDS reopened this Mar 21, 2017
@ErisDS
Copy link
Member

ErisDS commented Mar 21, 2017

This is not fixed, reopening

@ErisDS ErisDS mentioned this issue Mar 21, 2017
Closed
kirrg001 added a commit to kirrg001/Ghost that referenced this issue Mar 22, 2017
@kirrg001
🐛 fix image size timeout
4dcce35 
refs TryGhost#8041

- destroy socket is required, see https://nodejs.org/api/http.html#http_server_settimeout_msecs_callback
- optimise error messages in general
@kirrg001 kirrg001 mentioned this issue Mar 22, 2017
Merged
@kirrg001 kirrg001 added this to the 1.0.0 Beta Ready milestone Apr 3, 2017
@kirrg001 kirrg001 added server / core Issues relating to the server or core of Ghost bug [triage] something behaving unexpectedly labels Apr 3, 2017
ErisDS pushed a commit that referenced this issue Apr 4, 2017
@kirrg001 @ErisDS
🐛 fix image size timeout (#8210)
0d93078 
refs #8041

* 🐛  fix image size timeout

- destroy socket is required, see https://nodejs.org/api/http.html#http_server_settimeout_msecs_callback
- optimise error messages in general

* fix jshint

* Make timeouts configureable
kirrg001 added a commit to kirrg001/Ghost that referenced this issue Apr 5, 2017
@kirrg001
🐛 fix image size timeout
039c676 
refs TryGhost#8041

- destroy socket is required, see https://nodejs.org/api/http.html#http_server_settimeout_msecs_callback
- optimise error messages in general
- make timeouts configureable
@kirrg001 kirrg001 mentioned this issue Apr 5, 2017
Merged
ErisDS pushed a commit that referenced this issue Apr 5, 2017
@kirrg001 @ErisDS
🐛 fix image size timeout (#8283)
64d2a94 
closes #8041

- destroy socket is required, see https://nodejs.org/api/http.html#http_server_settimeout_msecs_callback
- optimise error messages in general
- make timeouts configureable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kirrg001 kirrg001

Labels
bug [triage] something behaving unexpectedly server / core Issues relating to the server or core of Ghost
Projects
None yet
Milestone
1.0.0 Beta Ready
Development

Successfully merging a pull request may close this issue.

🐛 fix image size timeout kirrg001/Ghost
5 participants
@kevinansfield @ErisDS @sebgie @kirrg001 @aileen