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

Error when fetching non-existent asset as first request after restart #6948

Closed
ErisDS opened this issue Jun 9, 2016 · 2 comments · Fixed by #6950 or #7548
Closed

Error when fetching non-existent asset as first request after restart #6948

ErisDS opened this issue Jun 9, 2016 · 2 comments · Fixed by #6950 or #7548
Labels
bug [triage] something behaving unexpectedly

Comments

@ErisDS
Copy link
Member

ErisDS commented Jun 9, 2016

If you start Ghost, and the first thing that you try to request is a non existent asset (e.g. something like /content/images/i-dont-exist.jpg) you will get an error:

Error: Cannot find module 'hbs'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at new View (/Users/hannah/Ghost/Ghost/node_modules/express/lib/view.js:78:30)
    at EventEmitter.render (/Users/hannah/Ghost/Ghost/node_modules/express/lib/application.js:569:12)
    at ServerResponse.render (/Users/hannah/Ghost/Ghost/node_modules/express/lib/response.js:961:7)
    at renderErrorInt (/Users/hannah/Ghost/Ghost/core/server/errors/index.js:323:36)
   ...

See this gist for the full stack trace.

In order to see this error you will need to be on a theme which doesn't call navigation in the error.hbs template, or where #6928 (which fixes that bug) is already merged.

This error occurs regardless of whether #6938 (moving hbs stuff around) is merged, which is expected because there is no functional change - just noting it here as a related change.

Steps to Reproduce

  1. Make sure your blog is running Casper
  2. Start (or restart) Ghost
  3. Don't request any other pages of Ghost yet!!
  4. Navigate straight to http://localhost:2368/content/images/i-dont-exist.jpg (or another image that doesn't exist for your blog
  5. 💥

This really should be rendering an error page!

Technical details:

  • Ghost Version: 0.8.0 / latest master
@ErisDS ErisDS added the bug [triage] something behaving unexpectedly label Jun 9, 2016
@kirrg001
Copy link
Contributor

kirrg001 commented Jun 9, 2016

fn configHbsForContext is not called in this case, that's why the whole hbs config is not initialised. i will care

kirrg001 added a commit to kirrg001/Ghost that referenced this issue Jun 9, 2016
issue #TryGhost#6948
- the hbs engine was never initialised  when server starts
- when you request a page which does not exist, express jumps directly into the error handlers
- delete some dynamic hbs engine setters in theme handler
kevinansfield pushed a commit that referenced this issue Jul 21, 2016
closes #6948
- the hbs engine was never initialised  when server starts
- when you request a page which does not exist, express jumps directly into the error handlers
- delete some dynamic hbs engine setters in theme handler
@kevinansfield
Copy link
Contributor

kevinansfield commented Jul 21, 2016

Re-opening, the merged/reverted PR had unintentional side-effects.

@kevinansfield kevinansfield reopened this Jul 21, 2016
chris-brown pushed a commit to chris-brown/Ghost that referenced this issue Aug 14, 2016
closes TryGhost#6948
- the hbs engine was never initialised  when server starts
- when you request a page which does not exist, express jumps directly into the error handlers
- delete some dynamic hbs engine setters in theme handler
@ErisDS ErisDS added the themes label Oct 5, 2016
kirrg001 pushed a commit that referenced this issue Oct 13, 2016
closes #4172, closes #6948, refs #7491, refs #7488, refs #7542, refs #7484

* 🎨 Co-locate all admin-related code in /admin
- move all the admin related code from controllers, routes and helpers into a single location
- add error handling middleware explicitly to adminApp
- re-order blogApp middleware to ensure the shared middleware is mounted after the adminApp
- TODO: rethink the structure of /admin, this should probably be an internal app

* 💄 Group global middleware together

- There are only a few pieces of middleware which are "global"
- These are needed for the admin, blog and api
- Everything else is only needed in one or two places

* ✨ Introduce a separate blogApp

- create a brand-new blogApp
- mount all blog/theme only middleware etc onto blogApp
- mount error handling on blogApp only

* 🎨 Separate error handling for HTML & API JSON

- split JSON and HTML error handling into separate functions
- re-introduce a way to not output the stack for certain errors
- add more tests around errors & an assertion framework for checking JSON Errors
- TODO: better 404 handling for static assets

Rationale:

The API is very different to the blog/admin panel:
 - It is intended to only ever serve JSON, never HTML responses
 - It is intended to always serve JSON

Meanwhile the blog and admin panel have no need for JSON errors,
when an error happens on those pages, we should serve HTML pages
which are nicely formatted with the error & using the correct template

* 🐛 Fix checkSSL to work for subapps

- in order to make this work on a sub app we need to use the pattern `req.originalUrl || req.url`

* 🔥 Get rid of decide-is-admin (part 1/2)

- delete decide-is-admin & tests
- add two small functions to apiApp and adminApp to set res.isAdmin
- mount checkSSL on all the apps
- TODO: deduplicate the calls to checkSSL by making blogApp a subApp :D
- PART 2/2: finish cleaning this up by removing it from where it's not needed and giving it a more specific name

Rationale:

Now that we have both an adminApp and an apiApp,
we can temporarily replace this weird path-matching middleware
with middleware that sets res.isAdmin for api & admin

* 🎨 Wire up prettyURLs on all Apps

- prettyURLs is needed for all requests
- it cannot be global because it has to live after asset middleware, and before routing
- this does not result in duplicate redirects, but does result in duplicate checks
- TODO: resolve extra middleware in stack by making blogApp a sub app

* ⏱ Add debug to API setup

* 🎨 Rename blogApp -> parentApp in middleware

* 🎨 Co-locate all blog-related code in /blog

- Move all of the blogApp code from middleware/index.js to blog/app.js
- Move routes/frontend.js to blog/routes.js
- Remove the routes/index.js and routes folder, this is empty now!
- @todo is blog the best name for this? 🤔
- @todo sort out the big hunk of asset-related mess
- @todo also separate out the concept of theme from blog

* 🎉 Replace middleware index with server/app.js

- The final piece of the puzzle! 🎉 🎈 🎂
- We no longer have our horrendous middleware/index.js
- Instead, we have a set of app.js files, which all use a familiar pattern

* 💄 Error handling fixups
mixonic pushed a commit to mixonic/Ghost that referenced this issue Oct 28, 2016
closes TryGhost#4172, closes TryGhost#6948, refs TryGhost#7491, refs TryGhost#7488, refs TryGhost#7542, refs TryGhost#7484

* 🎨 Co-locate all admin-related code in /admin
- move all the admin related code from controllers, routes and helpers into a single location
- add error handling middleware explicitly to adminApp
- re-order blogApp middleware to ensure the shared middleware is mounted after the adminApp
- TODO: rethink the structure of /admin, this should probably be an internal app

* 💄 Group global middleware together

- There are only a few pieces of middleware which are "global"
- These are needed for the admin, blog and api
- Everything else is only needed in one or two places

* ✨ Introduce a separate blogApp

- create a brand-new blogApp
- mount all blog/theme only middleware etc onto blogApp
- mount error handling on blogApp only

* 🎨 Separate error handling for HTML & API JSON

- split JSON and HTML error handling into separate functions
- re-introduce a way to not output the stack for certain errors
- add more tests around errors & an assertion framework for checking JSON Errors
- TODO: better 404 handling for static assets

Rationale:

The API is very different to the blog/admin panel:
 - It is intended to only ever serve JSON, never HTML responses
 - It is intended to always serve JSON

Meanwhile the blog and admin panel have no need for JSON errors,
when an error happens on those pages, we should serve HTML pages
which are nicely formatted with the error & using the correct template

* 🐛 Fix checkSSL to work for subapps

- in order to make this work on a sub app we need to use the pattern `req.originalUrl || req.url`

* 🔥 Get rid of decide-is-admin (part 1/2)

- delete decide-is-admin & tests
- add two small functions to apiApp and adminApp to set res.isAdmin
- mount checkSSL on all the apps
- TODO: deduplicate the calls to checkSSL by making blogApp a subApp :D
- PART 2/2: finish cleaning this up by removing it from where it's not needed and giving it a more specific name

Rationale:

Now that we have both an adminApp and an apiApp,
we can temporarily replace this weird path-matching middleware
with middleware that sets res.isAdmin for api & admin

* 🎨 Wire up prettyURLs on all Apps

- prettyURLs is needed for all requests
- it cannot be global because it has to live after asset middleware, and before routing
- this does not result in duplicate redirects, but does result in duplicate checks
- TODO: resolve extra middleware in stack by making blogApp a sub app

* ⏱ Add debug to API setup

* 🎨 Rename blogApp -> parentApp in middleware

* 🎨 Co-locate all blog-related code in /blog

- Move all of the blogApp code from middleware/index.js to blog/app.js
- Move routes/frontend.js to blog/routes.js
- Remove the routes/index.js and routes folder, this is empty now!
- @todo is blog the best name for this? 🤔
- @todo sort out the big hunk of asset-related mess
- @todo also separate out the concept of theme from blog

* 🎉 Replace middleware index with server/app.js

- The final piece of the puzzle! 🎉 🎈 🎂
- We no longer have our horrendous middleware/index.js
- Instead, we have a set of app.js files, which all use a familiar pattern

* 💄 Error handling fixups
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this issue Nov 20, 2016
closes TryGhost#6948
- the hbs engine was never initialised  when server starts
- when you request a page which does not exist, express jumps directly into the error handlers
- delete some dynamic hbs engine setters in theme handler
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this issue Nov 20, 2016
closes TryGhost#4172, closes TryGhost#6948, refs TryGhost#7491, refs TryGhost#7488, refs TryGhost#7542, refs TryGhost#7484

* 🎨 Co-locate all admin-related code in /admin
- move all the admin related code from controllers, routes and helpers into a single location
- add error handling middleware explicitly to adminApp
- re-order blogApp middleware to ensure the shared middleware is mounted after the adminApp
- TODO: rethink the structure of /admin, this should probably be an internal app

* 💄 Group global middleware together

- There are only a few pieces of middleware which are "global"
- These are needed for the admin, blog and api
- Everything else is only needed in one or two places

* ✨ Introduce a separate blogApp

- create a brand-new blogApp
- mount all blog/theme only middleware etc onto blogApp
- mount error handling on blogApp only

* 🎨 Separate error handling for HTML & API JSON

- split JSON and HTML error handling into separate functions
- re-introduce a way to not output the stack for certain errors
- add more tests around errors & an assertion framework for checking JSON Errors
- TODO: better 404 handling for static assets

Rationale:

The API is very different to the blog/admin panel:
 - It is intended to only ever serve JSON, never HTML responses
 - It is intended to always serve JSON

Meanwhile the blog and admin panel have no need for JSON errors,
when an error happens on those pages, we should serve HTML pages
which are nicely formatted with the error & using the correct template

* 🐛 Fix checkSSL to work for subapps

- in order to make this work on a sub app we need to use the pattern `req.originalUrl || req.url`

* 🔥 Get rid of decide-is-admin (part 1/2)

- delete decide-is-admin & tests
- add two small functions to apiApp and adminApp to set res.isAdmin
- mount checkSSL on all the apps
- TODO: deduplicate the calls to checkSSL by making blogApp a subApp :D
- PART 2/2: finish cleaning this up by removing it from where it's not needed and giving it a more specific name

Rationale:

Now that we have both an adminApp and an apiApp,
we can temporarily replace this weird path-matching middleware
with middleware that sets res.isAdmin for api & admin

* 🎨 Wire up prettyURLs on all Apps

- prettyURLs is needed for all requests
- it cannot be global because it has to live after asset middleware, and before routing
- this does not result in duplicate redirects, but does result in duplicate checks
- TODO: resolve extra middleware in stack by making blogApp a sub app

* ⏱ Add debug to API setup

* 🎨 Rename blogApp -> parentApp in middleware

* 🎨 Co-locate all blog-related code in /blog

- Move all of the blogApp code from middleware/index.js to blog/app.js
- Move routes/frontend.js to blog/routes.js
- Remove the routes/index.js and routes folder, this is empty now!
- @todo is blog the best name for this? 🤔
- @todo sort out the big hunk of asset-related mess
- @todo also separate out the concept of theme from blog

* 🎉 Replace middleware index with server/app.js

- The final piece of the puzzle! 🎉 🎈 🎂
- We no longer have our horrendous middleware/index.js
- Instead, we have a set of app.js files, which all use a familiar pattern

* 💄 Error handling fixups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly
Projects
None yet
3 participants