Merge pull request #2010 from overleaf/ta-http-errors

Add More Http Errors

GitOrigin-RevId: d7aab42e1c7cec8757d6f2c6fc839a6c3f204351

services/web/app/src/Features/Errors/Errors.js

@@ -39,7 +39,14 @@ class V1ConnectionError extends BackwardCompatibleError {}
 
 class UnconfirmedEmailError extends BackwardCompatibleError {}
 
-class EmailExistsError extends BackwardCompatibleError {}
+class EmailExistsError extends OError {
+  constructor(options) {
+    super({
+      message: 'Email already exists',
+      ...options
+    })
+  }
+}
 
 class InvalidError extends BackwardCompatibleError {}
 

services/web/app/src/Features/Errors/HttpErrorController.js

@@ -1,7 +1,7 @@
 const logger = require('logger-sharelatex')
 const OError = require('@overleaf/o-error')
+const HttpErrors = require('@overleaf/o-error/http')
 const AuthenticationController = require('../Authentication/AuthenticationController')
-const HttpErrors = require('./HttpErrors')
 
 function renderHTMLError(statusCode, publicInfo, res) {
   res.status(statusCode)

services/web/app/src/Features/Errors/HttpErrors.js

@@ -1,23 +0,0 @@
-const OError = require('@overleaf/o-error')
-
-class HttpError extends OError {
-  constructor(options) {
-    super(options)
-    this.statusCode = options.statusCode || 500
-  }
-}
-
-class UnprocessableEntityError extends HttpError {
-  constructor(options) {
-    super({
-      message: 'Unprocessable Entity',
-      statusCode: 422,
-      ...options
-    })
-  }
-}
-
-module.exports = {
-  HttpError,
-  UnprocessableEntityError
-}

services/web/app/src/Features/User/UserController.js

@@ -30,7 +30,7 @@ const SudoModeHandler = require('../SudoMode/SudoModeHandler')
 const settings = require('settings-sharelatex')
 const Errors = require('../Errors/Errors')
 const OError = require('@overleaf/o-error')
-const HttpErrors = require('../Errors/HttpErrors')
+const HttpErrors = require('@overleaf/o-error/http')
 const EmailHandler = require('../Email/EmailHandler')
 
 module.exports = UserController = {
@@ -108,7 +108,7 @@ module.exports = UserController = {
     )
   },
 
-  updateUserSettings(req, res) {
+  updateUserSettings(req, res, next) {
     const user_id = AuthenticationController.getLoggedInUserId(req)
     logger.log({ user_id }, 'updating account settings')
     return User.findById(user_id, function(err, user) {
@@ -187,18 +187,26 @@ module.exports = UserController = {
           return UserUpdater.changeEmailAddress(user_id, newEmail, function(
             err
           ) {
-            if (err != null) {
-              let message
-              logger.err(
-                { err, user_id, newEmail },
-                'problem updaing users email address'
-              )
+            if (err) {
+              let errorData = {
+                message: 'problem updaing users email address',
+                info: { user_id, newEmail, public: {} }
+              }
               if (err instanceof Errors.EmailExistsError) {
-                message = req.i18n.translate('email_already_registered')
+                errorData.info.public.message = req.i18n.translate(
+                  'email_already_registered'
+                )
+                return next(
+                  new HttpErrors.ConflictError(errorData).withCause(err)
+                )
               } else {
-                message = req.i18n.translate('problem_changing_email_address')
+                errorData.info.public.message = req.i18n.translate(
+                  'problem_changing_email_address'
+                )
+                next(
+                  new HttpErrors.InternalServerError(errorData).withCause(err)
+                )
               }
-              return res.send(500, { message })
             }
             return User.findById(user_id, function(err, user) {
               if (err != null) {

services/web/app/src/Features/User/UserEmailsController.js

@@ -20,6 +20,7 @@ const UserEmailsConfirmationHandler = require('./UserEmailsConfirmationHandler')
 const { endorseAffiliation } = require('../Institutions/InstitutionsAPI')
 const logger = require('logger-sharelatex')
 const Errors = require('../Errors/Errors')
+const HttpErrors = require('@overleaf/o-error/http')
 
 module.exports = UserEmailsController = {
   list(req, res, next) {
@@ -186,16 +187,23 @@ module.exports = UserEmailsController = {
 
   _handleEmailError(error, req, res, next) {
     if (error instanceof Errors.UnconfirmedEmailError) {
-      return res.status(409).json({
-        message: 'email must be confirmed'
-      })
+      return next(
+        new HttpErrors.ConflictError({
+          info: {
+            public: { message: 'email must be confirmed' }
+          }
+        }).withCause(error)
+      )
     } else if (error instanceof Errors.EmailExistsError) {
-      return res.status(409).json({
-        message: req.i18n.translate('email_already_registered')
-      })
-    } else {
-      return next(error)
+      return next(
+        new HttpErrors.ConflictError({
+          info: {
+            public: { message: req.i18n.translate('email_already_registered') }
+          }
+        }).withCause(error)
+      )
     }
+    next(new HttpErrors.InternalServerError().withCause(error))
   }
 }
 function __guard__(value, transform) {

services/web/app/src/Features/User/UserGetter.js

@@ -209,7 +209,7 @@ module.exports = UserGetter = {
   ensureUniqueEmailAddress(newEmail, callback) {
     return this.getUserByAnyEmail(newEmail, function(error, user) {
       if (user != null) {
-        return callback(new Errors.EmailExistsError('alread_exists'))
+        return callback(new Errors.EmailExistsError())
       }
       return callback(error)
     })

services/web/npm-shrinkwrap.json

@@ -498,9 +498,9 @@
       }
     },
     "@overleaf/o-error": {
-      "version": "2.0.0",
-      "resolved": "https://registry.npmjs.org/@overleaf/o-error/-/o-error-2.0.0.tgz",
-      "integrity": "sha512-6/5/3sMafzM2MFEiiLn8TTe+kMUtvXhxJhodCnBtcYn+B8qO/WvYa2sHYdBaakK+oh8mEwHkN4sPxwKPsK+2Ig=="
+      "version": "2.1.0",
+      "resolved": "https://registry.npmjs.org/@overleaf/o-error/-/o-error-2.1.0.tgz",
+      "integrity": "sha512-Zd9sks9LrLw8ErHt/cXeWIkyxWAqNAvNGn7wIjLQJH6TTEEW835PWOhpch+hQwwWsTxWIx/JDj+IpZ3ouw925g=="
     },
     "@protobufjs/aspromise": {
       "version": "1.1.2",

services/web/package.json

@@ -26,7 +26,7 @@
     "format:fix": "prettier-eslint '**/*.js' --write"
   },
   "dependencies": {
-    "@overleaf/o-error": "^2.0.0",
+    "@overleaf/o-error": "^2.1.0",
     "archiver": "0.9.0",
     "async": "0.6.2",
     "backbone": "^1.3.3",

services/web/test/unit/src/Errors/HttpErrorControllerTests.js

@@ -5,7 +5,7 @@ const SandboxedModule = require('sandboxed-module')
 const MockResponse = require('../helpers/MockResponse')
 const MockRequest = require('../helpers/MockRequest')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
-const HttpErrors = require('../../../../app/src/Features/Errors/HttpErrors')
+const HttpErrors = require('@overleaf/o-error/http')
 
 describe('HttpErrorController', () => {
   beforeEach(() => {
@@ -28,7 +28,7 @@ describe('HttpErrorController', () => {
           .AuthenticationController,
         'logger-sharelatex': this.logger,
         './Errors': Errors,
-        './HttpErrors': HttpErrors
+        '@overleaf/o-error/http': HttpErrors
       }
     })
   })

services/web/test/unit/src/User/UserControllerTests.js

@@ -23,7 +23,7 @@ const { ObjectId } = require('mongojs')
 const assert = require('assert')
 const OError = require('@overleaf/o-error')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
-const HttpErrors = require('../../../../app/src/Features/Errors/HttpErrors')
+const HttpErrors = require('@overleaf/o-error/http')
 
 describe('UserController', function() {
   beforeEach(function() {
@@ -109,7 +109,7 @@ describe('UserController', function() {
           inc() {}
         },
         '../Errors/Errors': Errors,
-        '../Errors/HttpErrors': HttpErrors,
+        '@overleaf/o-error/http': HttpErrors,
         '../Email/EmailHandler': { sendEmail: sinon.stub() }
       }
     })

services/web/test/unit/src/User/UserGetterTests.js

@@ -322,7 +322,6 @@ describe('UserGetter', function() {
       return this.UserGetter.ensureUniqueEmailAddress(this.newEmail, err => {
         should.exist(err)
         expect(err).to.be.an.instanceof(Errors.EmailExistsError)
-        err.message.should.equal('alread_exists')
         return done()
       })
     })