chore(errors): make email-sending errors a 422 for new addresses

In 75815f2, we added an error for email-sending failure to some routes.
Quite reasonably, we made it a 500 on the assumption that it probably
indicated a problem somewhere in our email-sending infrastructure, but
it turns out that in most cases it indicates a user mistyped their email
address. That causes Sentry to alert us pretty noisily, so this change
seeks to return a 422 instead in cases where the email address is
unverified.

Some rationale behind that decision:

* We still want actual infrastructure errors to be a 500, so I opted to
  keep the 500 in place on `/account/login` for verified accounts.

* For unverified accounts in `/account/login`, `/account/create` and
  `POST /recovery_email`, I wanted something different to a 400 so that
  it was distinct from our regular validation errors. The description
  for 422 seemed like a good appropriation for that.

* I avoided adding a new errno because that would require a knock-on
  change in the content server, and we want to target this as a point
  release for train 123.

docs/api.md

@@ -273,7 +273,7 @@ for `code` and `errno` are:
   This email can not currently be used to login
 * `code: 400, errno: 150`:
   Can not resend email code to an email that does not belong to this account
-* `code: 500, errno: 151`:
+* `code: 422, errno: 151`:
   Failed to send email
 * `code: 400, errno: 152`:
   Invalid token verification code
@@ -538,7 +538,7 @@ by the following errors
 * `code: 400, errno: 144`:
   Email already exists
 
-* `code: 500, errno: 151`:
+* `code: 422, errno: 151`:
   Failed to send email
 
 
@@ -1888,7 +1888,7 @@ by the following errors
 * `code: 400, errno: 141`:
   Email already exists
 
-* `code: 500, errno: 151`:
+* `code: 422, errno: 151`:
   Failed to send email
 
 

lib/error.js

@@ -734,10 +734,20 @@ AppError.cannotResendEmailCodeToUnownedEmail = function () {
   })
 }
 
-AppError.cannotSendEmail = function () {
+AppError.cannotSendEmail = function (isNewAddress) {
+  let code, error
+
+  if (isNewAddress) {
+    code = 422
+    error = 'Unprocessable Entity'
+  } else {
+    code = 500
+    error = 'Internal Server Error'
+  }
+
   return new AppError({
-    code: 500,
-    error: 'Internal Server Error',
+    code,
+    error,
     errno: ERRNO.FAILED_TO_SEND_EMAIL,
     message: 'Failed to send email'
   })

lib/routes/account.js

@@ -305,7 +305,7 @@ module.exports = (log, db, mailer, Password, config, customs, signinUtils, push)
 
                 // show an error to the user, the account is already created.
                 // the user can come back later and try again.
-                throw error.cannotSendEmail()
+                throw error.cannotSendEmail(true)
               })
           }
         }

lib/routes/emails.js

@@ -642,7 +642,7 @@ module.exports = (log, db, mailer, config, customs, push) => {
               log.error({op: 'mailer.sendVerifySecondaryEmail', err: err})
               return db.deleteEmail(emailData.uid, emailData.normalizedEmail)
                 .then(() => {
-                  throw error.cannotSendEmail()
+                  throw error.cannotSendEmail(true)
                 })
             })
         }

lib/routes/utils/signin.js

@@ -177,6 +177,7 @@ module.exports = (log, config, customs, db, mailer)  => {
       const redirectTo = request.payload.redirectTo
       const resume = request.payload.resume
       const ip = request.app.clientAddress
+      const isUnverifiedAccount = ! accountRecord.primaryEmail.isVerified
 
       let sessions
 
@@ -255,7 +256,7 @@ module.exports = (log, config, customs, db, mailer)  => {
 
       function sendEmail() {
         // For unverified accounts, we always re-send the account verification email.
-        if (! accountRecord.primaryEmail.isVerified) {
+        if (isUnverifiedAccount) {
           return sendVerifyAccountEmail()
         }
         // If the session needs to be verified, send the sign-in confirmation email.
@@ -348,7 +349,7 @@ module.exports = (log, config, customs, db, mailer)  => {
         .catch(function (err) {
           log.error({op: 'mailer.confirmation.error', err: err})
 
-          throw error.cannotSendEmail()
+          throw error.cannotSendEmail(isUnverifiedAccount)
         })
       }
 

test/local/routes/account.js

@@ -533,9 +533,9 @@ describe('/account/create', () => {
 
     return runTest(route, mockRequest).then(assert.fail, (err) => {
       assert.equal(err.message, 'Failed to send email')
-      assert.equal(err.output.payload.code, 500)
+      assert.equal(err.output.payload.code, 422)
       assert.equal(err.output.payload.errno, 151)
-      assert.equal(err.output.payload.error, 'Internal Server Error')
+      assert.equal(err.output.payload.error, 'Unprocessable Entity')
     })
   })
 })

test/local/routes/emails.js

@@ -820,6 +820,7 @@ describe('/recovery_email', () => {
       })
         .catch((err) => {
           assert.equal(err.errno, 151, 'failed to send email error')
+          assert.equal(err.output.payload.code, 422)
           assert.equal(mockDB.createEmail.callCount, 1, 'call db.createEmail')
           assert.equal(mockDB.deleteEmail.callCount, 1, 'call db.deleteEmail')
           assert.equal(mockDB.deleteEmail.args[0][0], mockRequest.auth.credentials.uid, 'correct uid passed')