From 01dee59e0430ece3e2c7cd53e9e0a67628add671 Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 18 Aug 2016 10:35:01 -0700 Subject: [PATCH] fix(send): Don't retry send Summary: - There are some cases in which constantly retrying send can cause unexpected bugs like sending multiple times, so don't retry send at all - Make 429 a permanent error code Test Plan: Manual Reviewers: jackie, evan Reviewed By: jackie, evan Differential Revision: https://phab.nylas.com/D3177 --- spec/tasks/send-draft-task-spec.es6 | 9 --------- spec/tasks/syncback-model-task-spec.es6 | 2 +- src/flux/nylas-api.coffee | 2 +- src/flux/tasks/multi-send-to-individual-task.es6 | 5 +---- src/flux/tasks/send-draft-task.es6 | 4 ---- 5 files changed, 3 insertions(+), 19 deletions(-) diff --git a/spec/tasks/send-draft-task-spec.es6 b/spec/tasks/send-draft-task-spec.es6 index 5997ca084..9cc1a8237 100644 --- a/spec/tasks/send-draft-task-spec.es6 +++ b/spec/tasks/send-draft-task-spec.es6 @@ -377,15 +377,6 @@ describe('SendDraftTask', function sendDraftTask() { })); }); - it("retries on timeouts", () => { - const thrownError = new APIError({statusCode: NylasAPI.TimeoutErrorCodes[0], body: "err"}); - spyOn(NylasAPI, 'makeRequest').andReturn(Promise.reject(thrownError)); - - waitsForPromise(() => this.task.performRemote().then((status) => { - expect(status).toBe(Task.Status.Retry); - })); - }); - describe("checking the promise chain halts on errors", () => { beforeEach(() => { spyOn(NylasEnv, 'reportError'); diff --git a/spec/tasks/syncback-model-task-spec.es6 b/spec/tasks/syncback-model-task-spec.es6 index 0ca86c579..91b110891 100644 --- a/spec/tasks/syncback-model-task-spec.es6 +++ b/spec/tasks/syncback-model-task-spec.es6 @@ -168,7 +168,7 @@ describe('SyncbackModelTask', function syncbackModelTask() { it("retries on retry-able API errors", () => { jasmine.unspy(NylasAPI, "makeRequest"); - const err = new APIError({statusCode: 429}); + const err = new APIError({statusCode: 420}); spyOn(NylasAPI, "makeRequest").andReturn(Promise.reject(err)) performRemote((status) => { expect(status).toBe(Task.Status.Retry) diff --git a/src/flux/nylas-api.coffee b/src/flux/nylas-api.coffee index 37b0ab682..cf1e24df0 100644 --- a/src/flux/nylas-api.coffee +++ b/src/flux/nylas-api.coffee @@ -14,7 +14,7 @@ async = require 'async' # A 0 code is when an error returns without a status code. These are # things like "ESOCKETTIMEDOUT" TimeoutErrorCodes = [0, "ETIMEDOUT", "ESOCKETTIMEDOUT", "ECONNRESET", "ENETDOWN", "ENETUNREACH"] -PermanentErrorCodes = [400, 401, 402, 403, 404, 405, 500, "ENOTFOUND", "ECONNREFUSED", "EHOSTDOWN", "EHOSTUNREACH"] +PermanentErrorCodes = [400, 401, 402, 403, 404, 405, 429, 500, "ENOTFOUND", "ECONNREFUSED", "EHOSTDOWN", "EHOSTUNREACH"] CancelledErrorCode = [-123, "ECONNABORTED"] SampleTemporaryErrorCode = 504 diff --git a/src/flux/tasks/multi-send-to-individual-task.es6 b/src/flux/tasks/multi-send-to-individual-task.es6 index ed9ed8982..0378398bc 100644 --- a/src/flux/tasks/multi-send-to-individual-task.es6 +++ b/src/flux/tasks/multi-send-to-individual-task.es6 @@ -33,10 +33,7 @@ export default class MultiSendToIndividualTask extends Task { // dozens of these tasks. The `MultieSendSessionCloseTask` // accumulates and shows the errors. if (err instanceof APIError) { - if (NylasAPI.PermanentErrorCodes.includes(err.statusCode)) { - return Promise.resolve([Task.Status.Failed, err]); - } - return Promise.resolve(Task.Status.Retry); + return Promise.resolve([Task.Status.Failed, err]); } return Promise.resolve([Task.Status.Failed, err]); }); diff --git a/src/flux/tasks/send-draft-task.es6 b/src/flux/tasks/send-draft-task.es6 index 427cb0645..3f62557c5 100644 --- a/src/flux/tasks/send-draft-task.es6 +++ b/src/flux/tasks/send-draft-task.es6 @@ -239,10 +239,6 @@ export default class SendDraftTask extends BaseDraftTask { let message = err.message; if (err instanceof APIError) { - if (!NylasAPI.PermanentErrorCodes.includes(err.statusCode)) { - return Promise.resolve(Task.Status.Retry); - } - message = `Sorry, this message could not be sent. Please try again, and make sure your message is addressed correctly and is not too large.`; if (err.statusCode === 402 && err.body.message) { if (err.body.message.indexOf('at least one recipient') !== -1) {