Better IMAP Error handling

This commit is contained in:
Evan Morikawa 2016-07-13 15:57:44 -07:00
parent 0b9933b514
commit 8c33c4976f
6 changed files with 100 additions and 10 deletions

View file

@ -9,6 +9,7 @@ const {
DatabaseConnector,
SyncPolicy,
Provider,
Errors,
} = require('nylas-core');
const {GMAIL_CLIENT_ID, GMAIL_CLIENT_SECRET, GMAIL_REDIRECT_URL} = process.env;
@ -162,7 +163,8 @@ module.exports = (server) => {
reply(Serialization.jsonStringify(response));
})
.catch((err) => {
reply({error: err.message}).code(400);
const code = err instanceof Errors.IMAPAuthenticationError ? 401 : 400
reply({message: err.message, type: "api_error"}).code(code);
})
},
});
@ -204,13 +206,13 @@ module.exports = (server) => {
const oauthClient = new OAuth2(GMAIL_CLIENT_ID, GMAIL_CLIENT_SECRET, GMAIL_REDIRECT_URL);
oauthClient.getToken(request.query.code, (err, tokens) => {
if (err) {
reply({error: err.message}).code(400);
reply({message: err.message, type: "api_error"}).code(400);
return;
}
oauthClient.setCredentials(tokens);
google.oauth2({version: 'v2', auth: oauthClient}).userinfo.get((error, profile) => {
if (error) {
reply({error: error.message}).code(400);
reply({message: error.message, type: "api_error"}).code(400);
return;
}
@ -247,7 +249,7 @@ module.exports = (server) => {
reply(Serialization.jsonStringify(response));
})
.catch((connectionErr) => {
reply({error: connectionErr.message}).code(400);
reply({message: connectionErr.message, type: "api_error"}).code(400);
});
});
});

View file

@ -11,6 +11,12 @@ function jsonSchema(modelName) {
if (models.includes(modelName)) {
return Joi.object();
}
if (modelName === 'Error') {
return Joi.object().keys({
message: Joi.string(),
type: Joi.string(),
})
}
if (modelName === 'Account') {
// Ben: Disabled temporarily because folks keep changing the keys and it's hard
// to keep in sync. Might need to consider another approach to these.

View file

@ -5,6 +5,7 @@ const EventEmitter = require('events');
const IMAPBox = require('./imap-box');
const {
convertImapError,
IMAPConnectionNotReadyError,
IMAPConnectionEndedError,
} = require('./imap-errors');
@ -111,7 +112,7 @@ class IMAPConnection extends EventEmitter {
this._imap.once('error', (err) => {
this.end();
reject(err);
reject(convertImapError(err));
});
this._imap.once('end', () => {

View file

@ -1,21 +1,100 @@
/**
* Errors may come from:
*
* 1. Underlying IMAP provider (Fastmail, Yahoo, etc)
* 2. Node IMAP
* 3. K2 code
*
* NodeIMAP puts a `source` attribute on `Error` objects to indicate where
* a particular error came from. See https://github.com/mscdex/node-imap/blob/master/lib/Connection.js
*
* These may have the following values:
*
* - "socket-timeout": Created by NodeIMAP when `config.socketTimeout`
* expires on the base Node `net.Socket` and socket.on('timeout') fires
* Message: 'Socket timed out while talking to server'
*
* - "timeout": Created by NodeIMAP when `config.connTimeout` has been
* reached when trying to connect the socket.
* Message: 'Timed out while connecting to server'
*
* - "socket": Created by Node's `net.Socket` on error. See:
* https://nodejs.org/api/net.html#net_event_error_1
* Message: Various from `net.Socket`
*
* - "protocol": Created by NodeIMAP when `bad` or `no` types come back
* from the IMAP protocol.
* Message: Various from underlying IMAP protocol
*
* - "authentication": Created by underlying IMAP connection or NodeIMAP
* in a few scenarios.
* Message: Various from underlying IMAP connection
* OR: No supported authentication method(s) available. Unable to login.
* OR: Logging in is disabled on this server
*
* - "timeout-auth": Created by NodeIMAP when `config.authTimeout` has
* been reached when trying to authenticate
* Message: 'Timed out while authenticating with server'
*
*/
function convertImapError(imapError) {
let error;
switch(imapError.source) {
case "socket-timeout":
error = new IMAPConnectionTimeoutError(imapError); break;
case "timeout":
error = new IMAPConnectionTimeoutError(imapError); break;
case "socket":
error = new IMAPSocketError(imapError); break;
case "protocol":
error = new IMAPProtocolError(imapError); break;
case "authentication":
error = new IMAPAuthenticationError(imapError); break;
case "timeout-auth":
error = new IMAPAuthenticationTimeoutError(imapError); break;
default:
return error
}
error.source = imapError.source
return error
}
// "Source" is a hack so that the error matches the ones used by node-imap
/**
* An abstract base class that can be used to indicate errors that may fix
* themselves when retried
*/
class RetryableError extends Error { }
class IMAPConnectionNotReadyError extends Error {
/**
* Errors that originate from NodeIMAP. See `convertImapError` for
* documentation on underlying causes
*/
class IMAPSocketError extends RetryableError { }
class IMAPConnectionTimeoutError extends RetryableError { }
class IMAPAuthenticationTimeoutError extends RetryableError { }
class IMAPProtocolError extends Error { }
class IMAPAuthenticationError extends Error { }
class IMAPConnectionNotReadyError extends RetryableError {
constructor(funcName) {
super(`${funcName} - You must call connect() first.`);
this.source = 'socket';
}
}
class IMAPConnectionEndedError extends Error {
constructor(msg = "The IMAP Connection was ended.") {
super(msg);
this.source = 'socket';
}
}
module.exports = {
convertImapError,
RetryableError,
IMAPSocketError,
IMAPConnectionTimeoutError,
IMAPAuthenticationTimeoutError,
IMAPProtocolError,
IMAPAuthenticationError,
IMAPConnectionNotReadyError,
IMAPConnectionEndedError,
};

View file

@ -12,4 +12,5 @@ module.exports = {
SchedulerUtils: require('./scheduler-utils'),
MessageTypes: require('./message-types'),
Logger: require('./logger'),
Errors: require('./imap-errors'),
}

View file

@ -4,6 +4,7 @@ const {
PubsubConnector,
DatabaseConnector,
MessageTypes,
Errors,
} = require('nylas-core');
const {
jsonError,
@ -200,7 +201,7 @@ class SyncWorker {
this.closeConnection()
// Continue to retry if it was a network error
if (error.source && (error.source.includes('socket') || error.source.includes('timeout'))) {
if (error instanceof Errors.RetryableError) {
return Promise.resolve()
}