0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-15 03:01:37 -05:00

Improved error handling for SQL errors ()

refs 

- 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.
This commit is contained in:
Chris Raible 2023-11-01 13:47:41 -07:00 committed by GitHub
parent a382cd8a91
commit 9a8c703e34
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 47 additions and 2 deletions
ghost
core/core/shared
mw-error-handler

View file

@ -22,13 +22,25 @@ if (sentryConfig && !sentryConfig.disabled) {
event.exception.values[0].type = exception.context;
}
// This is a mysql2 error — add some additional context
if (exception.sql) {
event.exception.values[0].type = `SQL Error ${exception.errno}: ${exception.sqlErrorCode}`;
event.exception.values[0].value = exception.sqlMessage;
event.contexts.mysql = {
errno: exception.errno,
code: exception.sqlErrorCode,
sql: exception.sql,
message: exception.sqlMessage,
state: exception.sqlState
};
}
// This is a Ghost Error, copy all our extra data to tags
event.tags.type = exception.errorType;
event.tags.code = exception.code;
event.tags.id = exception.id;
event.tags.statusCode = exception.statusCode;
}
return event;
}
});

View file

@ -87,7 +87,17 @@ module.exports.prepareError = (err, req, res, next) => {
message: err.message,
statusCode: err.statusCode
});
// For everything else, create a generic 500 error, with context set to the original error message
// Catch database errors and turn them into 500 errors, but log some useful data to sentry
} else if (isDependencyInStack('mysql2', err)) {
// we don't want to return raw database errors to our users
err.sqlErrorCode = err.code;
err = new errors.InternalServerError({
err: err,
message: tpl(messages.genericError),
statusCode: err.statusCode,
code: 'UNEXPECTED_ERROR'
});
// For everything else, create a generic 500 error, with context set to the original error message
} else {
err = new errors.InternalServerError({
err: err,

View file

@ -104,6 +104,29 @@ describe('Prepare Error', function () {
done();
});
});
it('Correctly prepares a mysql2 error', function (done) {
let error = new Error('select anything from anywhere where something = anything;');
error.stack += '\n';
error.stack += path.join('node_modules', 'mysql2', 'lib');
error.code = 'ER_WRONG_VALUE';
error.sql = 'select anything from anywhere where something = anything;';
error.sqlMessage = 'Incorrect DATETIME value: 3234234234';
prepareError(error, {}, {
set: () => {}
}, (err) => {
err.statusCode.should.eql(500);
err.name.should.eql('InternalServerError');
err.message.should.eql('An unexpected error occurred, please try again.');
err.code.should.eql('UNEXPECTED_ERROR');
err.sqlErrorCode.should.eql('ER_WRONG_VALUE');
err.sql.should.eql('select anything from anywhere where something = anything;');
err.sqlMessage.should.eql('Incorrect DATETIME value: 3234234234');
done();
});
});
});
describe('Prepare Stack', function () {