fix https://linear.app/tryghost/issue/SLO-85/fix-http-500-on-contentposts
- in the event we give the incorrect format in a filter, MySQL will
throw an error and we'll throw a HTTP 500 error
- we can capture this error and return a more useful error to the user
- ideally we'd do this in a validation step before attempting the query,
but parsing this out of NQL and detecting which columns are DATETIME
could be quite tricky
fix https://linear.app/tryghost/issue/SLO-96/invalid-version-must-be-a-string-got-type-object-an-unexpected-error
- in the event that a non-semver Accept-Version header is given, the
current code will throw an error because the semver lib can't compare null
against a valid version
- the error in question is `Must be a string. Got type "object"`
- to fix this, we can just detect a null and early return with a
BadRequestError
- also adds a breaking test
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.
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
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
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
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)
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
- 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
- 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!
- 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
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)
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.
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.
- 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
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
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.
