Commit Graph

7 Commits

Author SHA1 Message Date
Hannah Wolfe
8c93bdff41
Removed unnecessary BasicErrorRenderer
refs: 4474ca1a1d
refs: 0799f02e80

The BasicErrorRenderer was created as a fallback for when we needed to not render templates, which is
chiefly when we're trying to render a 404 for an image. Using a template puts us at risk of an infinite 404 loop
if the missing image is referenced in the 404 template.

As of 0799f02e, the HTMLErrorRenderer no longer uses templates - instead we serve a very simple HTML page.
This can be used instead of the BasicErrorRenderer, as it results in a properly formatted error.
Even when sending responses in plain text, the content type is returned as HTML and therefore having an
unformatted error makes no sense - if we really need a non-html format I guess there should be no body at all.
2021-11-29 15:40:55 +00:00
Hannah Wolfe
303ea87897
Removed overly-specific handling of handlebars errors
refs: https://github.com/TryGhost/Ghost/issues/10496
refs: https://github.com/handlebars-lang/handlebars.js/issues/1548

- For a while, handlebars would and could generate quite a lot of obscure sytnax error messages for malformed helpers that we couldn't catch in gscan
- This was fixed in https://github.com/handlebars-lang/handlebars.js/issues/1548
- We have also since added AST parsing to gscan, so if there are more obscure syntax errors, we should be able to add rules to catch them
- Therefore, this additional, overly specific code block isn't needed anymore, so let's clean it up!
2021-11-25 14:53:36 +00:00
Hannah Wolfe
5e931e2e37
Added err.context & err.help to stack traces
- When we handle errors in Ghost, we are supposed to use a pattern of supplying 3 messages:
   - message: what went wrong
   - context: details about why how or where the error happened
   - help: where the user can go to get help with this error
- We do this in many places and our JSON error handler and CLI error logging tools are designed to output this extra information
- However, stack traces, which start with message as the first line and then output the stack are totally missing this
- By injecting the additional messages into the stack once an error has been "ghostified" we should get clearer messages everywhere

Notes:
- I've additionally injected a "Stack Trace:" line that makes it easier to read the error vs the stack
- This code looks a little weird because the lines are inserted backwards, but that allows us to always to the insert at position 1 as per the comment,
   so we don't have to keep track of whether we already injected something or not
2021-11-24 13:01:06 +00:00
Hannah Wolfe
0799f02e80
Rewritten HTMLErrorRenderer w/o asset helper & template
refs: 2af9e2e12

- This new HTMLErrorRenderer is borrowed heavily from finalHandler
   - This is the module that express uses to render errors if there is no custom errorhandler
- It just renders a really simple html page wrapping err.stack in a <pre>
   - This results in a nicely formatted, but unstyled error page
- I also updated BasicErrorRenderer to use the same res.statusCode + err.stack pattern rather than err.message

Note: This error renderer is _only_ used for renderering errors on the `/ghost/` route
 - In almost all cases, errors here are rendered by Ember
 - The only error that can be rendered here is a missing template error see: 2af9e2e12
2021-11-24 13:01:06 +00:00
Hannah Wolfe
0c4c9ce553
Moved JSON error handlers next to each other
- helps for comparing and contrasting
- context is largely lost on why we have these two, need to figure it out and if possible, simplify
2021-11-24 11:42:53 +00:00
Hannah Wolfe
f417c4c732
Merged our two maintenance middleware into one
- Reduced our maintenance middleware code down to the bare minimum!
  - We have an old maintenance middleware in place to handle when a site is forcibly put into maintenance mode, or the urlService hasn't finished booting
    - This maintenance middleware was mounted on every sub app, instead of globally for reasons I no longer remember
  - Recently, we introduced a new, static version of maintenence middleware to show during the boot process so we can get the server started earlier & not drop requests
    - This version has its own HTML template and doesn't depend on any of Ghost's error rendering code
  - To simplify and help with decoupling, this commit merges the two middleware, so that the new independent & static middleware renders its template for any one of the 3 possible maintenance modes
    - It only needs to exist in the top level app 🙌

TODO: move the maintenance middleware to its own file/package so it's not part of the app.js as that is weird
2021-11-24 11:27:18 +00:00
Hannah Wolfe
4f9b72ff43
Renamed middlewares to middleware consistently
- This is a minor bugbare, but it will affect some configuration I'm about to do for c8
- I've been wanting to do it for ages, middleware is plural all on it's own so it's an odd affectation in our codebase
- This also only exists in 2 places, everywhere else we use "middleware"
- Sadly it did result in a lot of churn as I did a full find and replace, but consistency is king!
2021-11-16 15:51:47 +00:00