Commit Graph

21 Commits

Author SHA1 Message Date
Chris Raible
9a8c703e34
Improved error handling for SQL errors (#18797)
refs TryGhost/Product#4083

- In the vast majority of cases, we shouldn't have SQL errors in our
code. Due to some limitations with validating e.g. nql filters passed to
the API, sometimes we don't catch these errors and they bubble up to the
user.
- In these rare cases, Ghost was returning the raw SQL error from mysql
which is not very user friendly and also exposes information about the
database, which generally is not a good practice.
- To make things worse, Sentry was treating every instance of these
errors as a unique issue, even when it was exactly the same query
failing over and over.
- This change improves the error message returned from the API, and also
makes sure that Sentry will group all these errors together, so we can
easily see how many times they are happening and where.
- It also adds more specific context to the event that is sent to
Sentry, including the mysql error number, code, and the SQL query
itself.
2023-11-01 13:47:41 -07:00
Hannah Wolfe
62cd52ff98 Improved Sentry server side error reporting
refs: https://github.com/TryGhost/Team/issues/1121
refs: 54574025e0

- The previous change to fall back to a generic error on the server side is resulting in lots of much less useful Sentry reports
- For unexpected errors, change what's sent to Sentry back to context
- This is done by adding a specific code, so we don't have to match on a string that might change
- Also add the error type, id, code & statusCode as tags to the events - these are searchable structured data
- Adding code as a tag also makes it possible to find all errors that showed the generic message
2022-11-23 12:37:24 +00:00
Hannah Wolfe
682f3a2014 Downgraded express-hbs errors to 400
refs: https://github.com/TryGhost/Team/issues/2289
refs: https://github.com/TryGhost/express-hbs/issues/161

- Themes that resuse layouts as templates trigger horrible errors, which are thrown as 500s
- But there's nothing the server is doing wrong, it's a theme user, so we downgrade these to 400s
- There is more to do here to improve the errors shown, but this is just a first step to ensure that theme issues don't look like server failures
2022-11-22 18:20:33 +00:00
Hannah Wolfe
54574025e0 Improved server-side error handling
refs: https://github.com/TryGhost/Team/issues/1121
refs: dfffa309a8

- This makes a fundamental change to Ghost's server side error handling, so that no unhandled errors are used as API responses
- Anything that has been handled and rethrown as a Ghost error cna be trusted
- We also already trust a couple of known errors from bookshelf and handlebars
- Everything else is assumed to be a code error, and should not be shown as the main message
- Instead we use our generic fallback message and use the OG error as context
2022-11-21 12:13:16 +00:00
Naz
7b009bf1fe Enabled shared caching of 404 error responses
refs https://github.com/TryGhost/Toolbox/issues/410

- The 'private' value in 'Cache-Control' response header for all errors made it impossible for shared caches (e.g.: Fastly, Cloudflare) to cache 404 responses efficiently.
- The change substitutes 'max-age=0' which should not effect the browser cache behavior but would allow shared caches to process such requests efficiently.
- A more loose caching logic only applies to 404 responses from GET requests that are not user-specific (non-authenticated, non-cookie containing requests)
2022-09-26 14:54:50 +08:00
Naz
2acb0fca74 Refactored error cache control logic to middleware
refs https://github.com/TryGhost/Toolbox/issues/410

- This is groundwork for split cache-control rules for Admin app endpoints and the rest of Ghost apps.
2022-09-26 14:54:50 +08:00
Hannah Wolfe
af94855349 Removed bluebird catch predicates from API endpoints
refs: https://github.com/TryGhost/Ghost/issues/14882

- I found a common pattern where catch predicates were being used to catch non-existent models in destroy methods, and sometimes elsewhere in the API endpoints
- The use of predicates is deprecated, and we're working to remove them from everywhere, so that we can remove bluebird
- In order to still handle these errors correctly, we needed a small change to mw-error-handler so that it can detect EmptyResponse errors from bookshelf, as well as 404s
Note: there is a small change as a result of this - the context on these errors now says "Resource not found" instead of "{ModelName} not found".
- I think this is acceptable for now, as we will be reviewing these errors in more depth later. It's quite easy to make changes, we just have to decide what with proper design input
2022-08-24 11:27:09 +01:00
Hannah Wolfe
0ad0cee19b Removed unused jsonErrorRenderer + renamed V2
- As of Ghost 5.0 we only use the V2 version of jsonErrorRenderer
- Removed the old one, and renamed the V2 to not have a suffix any more
- Added 100% coverage to tests whilst here
2022-05-06 15:45:41 +01:00
Hannah Wolfe
9e6f5c5194 Updated copy for accept-version errors
- Copy has been updated and approved now :)
2022-05-04 13:04:40 +01:00
Hannah Wolfe
e4af12e5c0 Added additional ghostErrorCode note for the major
refs: https://github.com/TryGhost/Toolbox/issues/316

- getting ready to do a major bump
2022-05-02 14:45:16 +01:00
Hannah Wolfe
fc9e61ff35 Fixed linting error
- trying to call new RequestNotAcceptableError with a variable triggers a lint warning in newer versions of eslint-plugin-ghost
- this workaround is worth it for the safety of not allowing single strings to be passed in!
2022-05-02 13:47:29 +01:00
Hannah Wolfe
a6e204b829 Deprecated handleJSONResponse
- this middleware block is used in v4 but not in v5
- we want to remove it and then rename handleJSONResponseV2 so that we have one single consistent error handling block
2022-05-02 13:43:48 +01:00
Naz
258b0acc51 Added specific 'code' properties for RequestNotAcceptableError
refs https://github.com/TryGhost/Toolbox/issues/292

- There's a need to distinguish different types of RequestNotAcceptableError erros by their code. The code is also having an instructional name to give it more explicit utility (nice clue for a developer seeing the error)
2022-04-21 15:57:43 +08:00
Naz
ebc8881d65 Renamed variables to match their content
refs https://github.com/TryGhost/Toolbox/issues/280

- The naming of variables should've been vice-versa. Should make the code less confusing now
2022-04-13 13:30:16 +08:00
Naz
ee7655213e Added 406 response for outdated Ghost instance
refs https://github.com/TryGhost/Toolbox/issues/280

- When an outdated client receives a 404 as a response there's no clear way to act on it. Plain 404 says nothing about need to update.
- In such cases the resourceNotFound handler should return a 406 error indicating the Ghost instance needs an update.
2022-04-08 12:52:41 +08:00
Naz
88500e0704 Added 406 response for outdated API clients
refs https://github.com/TryGhost/Toolbox/issues/280

- When an outdated client receives a 404 as a response there's no clear way to act on it. Plain 404 says nothing about need to update.
- In such cases the resourceNotFound handler should return a 406 error indicating the client need to update.
2022-04-08 12:40:37 +08:00
Daniel Lockyer
10e97cad23 Handled unknown errors when preparing user message
- in the event we get an unknown error bubble up, we don't handle the
  templating on the error name
- `@tryghost/tpl` throws an error because we pass an undefined string:
  `Cannot read properties of undefined (reading 'replace')`
- this commit adds handling to fallback to a different user message in
  that event so we don't cause a 500 error
2022-03-24 10:06:55 +00:00
Sam Lord
d65ba072f9 Split prepareError from prepareStack
no issue

Change to error handling caused all theme errors to be reported in Sentry, this fix (and a respective fix in Ghost) allows the error to be prepared for sentry before replacing the stack
2022-03-21 09:38:05 +00:00
Sam Lord
51588c6a24 Ensure Sentry is triggered before replacing the stack trace
refs: https://github.com/TryGhost/Team/issues/1369

If we prepare the error for users to view before using Sentry, then the error passed to Sentry will have the stack trace removed for production environments.

@tryghost/errors@1.2.5 also made it so that the error is not mutated, but cloned and a new one is returned.
2022-03-11 10:47:26 +00:00
Sam Lord
6a5ea251a5 Fix usage of member function for errors
no issue
2021-12-14 14:07:07 +00:00
Sam Lord
4ef7c974a3 Add @tryghost/mw-error-handler
refs: https://github.com/TryGhost/Toolbox/issues/137
Package includes same logic as was in the Ghost codebase but needs Sentry injected
2021-12-07 18:40:46 +00:00