Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00
Code Issues Activity

Improved log output for welcome email error (#9016)

Browse source 
* Improved log output for welcome email error

no issue

- if Ghost is unable to send a welcome email, the server log printe a huge error log
- the reason was that each component wrapped the original error into a new error instance
  - so the stack grows and grows
- the golden rule should always be: the smallest/lowest component should instanitate a specifc error
  - the caller can expect to receive a custom Ghost error

* Tidy up error messages for mail failures and fix tests

- We never use "Error:" notation in our translations
- Make the error messages consistent and show a reason if possible
This commit is contained in:
Katharina Irrgang 2017-09-19 15:24:20 +02:00 • committed by Hannah Wolfe
parent ca53a83569
commit edf2348394
5 changed files with 49 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
core/server/api/authentication.js
View file

@ -532,13 +532,11 @@ authentication = {
}]
};
mailAPI.send(payload, {context: {internal: true}}).catch(function (error) {
logging.error(new errors.EmailError({
err: error,
context: i18n.t('errors.api.authentication.unableToSendWelcomeEmail'),
help: i18n.t('errors.api.authentication.checkEmailConfigInstructions', {url: 'http://docs.ghost.org/v1/docs/mail-config'})
}));
});
mailAPI.send(payload, {context: {internal: true}})
.catch(function (err) {
err.context = i18n.t('errors.api.authentication.unableToSendWelcomeEmail');
logging.error(err);
});
})
.return(setupUser);
}

3
core/server/api/mail.js
View file

@ -5,7 +5,6 @@ var Promise = require('bluebird'),
pipeline = require('../utils/pipeline'),
apiUtils = require('./utils'),
models = require('../models'),
errors = require('../errors'),
i18n = require('../i18n'),
mail = require('../mail'),
notificationsAPI = require('./notifications'),
@ -36,7 +35,7 @@ function sendMail(object) {
);
}
return Promise.reject(new errors.EmailError({err: err}));
return Promise.reject(err);
});
}

54
core/server/mail/GhostMailer.js
View file

@ -1,12 +1,13 @@
// # Mail
// Handles sending email for Ghost
var _ = require('lodash'),
Promise = require('bluebird'),
validator = require('validator'),
config = require('../config'),
var _ = require('lodash'),
Promise = require('bluebird'),
validator = require('validator'),
config = require('../config'),
errors = require('../errors'),
settingsCache = require('../settings/cache'),
i18n = require('../i18n'),
utils = require('../utils');
i18n = require('../i18n'),
utils = require('../utils');
function GhostMailer() {
var nodemailer = require('nodemailer'),
@ -47,14 +48,19 @@ GhostMailer.prototype.getDomain = function () {
// This assumes that api.settings.read('email') was already done on the API level
GhostMailer.prototype.send = function (message) {
var self = this,
to;
to,
help = i18n.t('errors.api.authentication.checkEmailConfigInstructions', {url: 'http://docs.ghost.org/v1/docs/mail-config'}),
errorMessage = i18n.t('errors.mail.failedSendingEmail.error');
// important to clone message as we modify it
message = _.clone(message) || {};
to = message.to || false;
if (!(message && message.subject && message.html && message.to)) {
return Promise.reject(new Error(i18n.t('errors.mail.incompleteMessageData.error')));
return Promise.reject(new errors.EmailError({
message: i18n.t('errors.mail.incompleteMessageData.error'),
help: help
}));
}
message = _.extend(message, {
@ -65,9 +71,15 @@ GhostMailer.prototype.send = function (message) {
});
return new Promise(function (resolve, reject) {
self.transport.sendMail(message, function (error, response) {
if (error) {
return reject(new Error(error));
self.transport.sendMail(message, function (err, response) {
if (err) {
errorMessage += i18n.t('errors.mail.reason', {reason: err.message || err});
return reject(new errors.EmailError({
message: errorMessage,
err: err,
help: help
}));
}
if (self.transport.transportType !== 'DIRECT') {
@ -75,23 +87,25 @@ GhostMailer.prototype.send = function (message) {
}
response.statusHandler.once('failed', function (data) {
var reason = i18n.t('errors.mail.failedSendingEmail.error');
if (data.error && data.error.errno === 'ENOTFOUND') {
reason += i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain});
errorMessage += i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain});
}
reason += '.';
return reject(new Error(reason));
return reject(new errors.EmailError({
message: errorMessage,
help: help
}));
});
response.statusHandler.once('requeue', function (data) {
var errorMessage = i18n.t('errors.mail.messageNotSent.error');
if (data.error && data.error.message) {
errorMessage += i18n.t('errors.general.moreInfo', {info: data.error.message});
errorMessage += i18n.t('errors.mail.reason', {reason: data.error.message});
}
return reject(new Error(errorMessage));
return reject(new errors.EmailError({
message: errorMessage,
help: help
}));
});
response.statusHandler.once('sent', function () {

11
core/server/translations/en.json
View file

@ -167,7 +167,6 @@
},
"general": {
"maintenance": "Ghost is currently undergoing maintenance, please wait a moment then retry.",
"moreInfo": "\nMore info: {info}",
"requiredOnFuture": "This will be required in future. Please see {link}",
"internalError": "Something went wrong."
},
@ -185,17 +184,15 @@
},
"mail": {
"incompleteMessageData": {
"error": "Error: Incomplete message data."
"error": "Incomplete message data."
},
"failedSendingEmail": {
"error": "Error: Failed to send email"
"error": "Failed to send email."
},
"noMailServerAtAddress": {
"error": " - no mail server found at {domain}"
"error": " No mail server found at {domain}."
},
"messageNotSent": {
"error": "Error: Message could not be sent"
}
"reason": " Reason: {reason}."
},
"models": {
"subscriber": {

10
core/test/unit/mail/GhostMailer_spec.js
View file

@ -96,7 +96,7 @@ describe('Mail: Ghostmailer', function () {
mailer.send(mailDataNoServer).then(function () {
done(new Error('Stub did not error'));
}).catch(function (error) {
error.message.should.eql('Error: Stub made a boo boo :(');
error.message.should.containEql('Stub made a boo boo :(');
done();
}).catch(done);
});
@ -113,7 +113,7 @@ describe('Mail: Ghostmailer', function () {
descriptors.forEach(function (d) {
d.isFulfilled().should.be.false();
d.reason().should.be.an.instanceOf(Error);
d.reason().message.should.eql('Error: Incomplete message data.');
d.reason().message.should.eql('Incomplete message data.');
});
done();
}).catch(done);
@ -136,7 +136,7 @@ describe('Mail: Ghostmailer', function () {
mailer.send(mailDataNoDomain).then(function () {
done(new Error('Error message not shown.'));
}, function (error) {
error.message.should.startWith('Error: Failed to send email');
error.message.should.startWith('Failed to send email.');
done();
}).catch(done);
});
@ -147,7 +147,7 @@ describe('Mail: Ghostmailer', function () {
mailer.send(mailDataNoServer).then(function () {
done(new Error('Error message not shown.'));
}, function (error) {
error.message.should.eql('Error: Failed to send email.');
error.message.should.eql('Failed to send email.');
done();
}).catch(done);
});
@ -158,7 +158,7 @@ describe('Mail: Ghostmailer', function () {
mailer.send(mailDataIncomplete).then(function () {
done(new Error('Error message not shown.'));
}, function (error) {
error.message.should.eql('Error: Incomplete message data.');
error.message.should.eql('Incomplete message data.');
done();
}).catch(done);
});