Merge pull request #2984 from overleaf/msm-oerror-remove-bad-request-error

Replace HTTPErrors.BadRequestError with calls to badRequest() handler

GitOrigin-RevId: 57a91a13bde942ee373e235ee925f1c76a0f4e88

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

@@ -175,6 +175,12 @@ class DocHasRangesError extends OError {
   }
 }
 
+class InvalidQueryError extends OError {
+  constructor(options) {
+    super({ message: 'invalid search query', ...options })
+  }
+}
+
 module.exports = {
   OError,
   BackwardCompatibleError,
@@ -203,5 +209,6 @@ module.exports = {
   ProjectNotFoundError,
   UserNotFoundError,
   UserNotCollaboratorError,
-  DocHasRangesError
+  DocHasRangesError,
+  InvalidQueryError
 }

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

@@ -1,7 +1,7 @@
 const logger = require('logger-sharelatex')
 
 function renderJSONError(res, message, info) {
-  const fullInfo = { message, ...info }
+  const fullInfo = { ...info, message }
   if (info.message) {
     logger.warn(
       info,
@@ -12,6 +12,21 @@ function renderJSONError(res, message, info) {
 }
 
 module.exports = {
+  badRequest(req, res, message, info) {
+    res.status(400)
+    switch (req.accepts(['html', 'json'])) {
+      case 'html':
+        return res.render('general/400', {
+          title: 'Client Error',
+          message: message
+        })
+      case 'json':
+        return renderJSONError(res, message, info || {})
+      default:
+        return res.send('client error')
+    }
+  },
+
   forbidden(req, res, message = 'restricted', info = {}) {
     res.status(403)
     switch (req.accepts(['html', 'json'])) {

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

@@ -14,6 +14,55 @@ describe('HttpErrorHandler', function() {
     })
   })
 
+  describe('badRequest', function() {
+    it('returns 400', function() {
+      this.HttpErrorHandler.badRequest(this.req, this.res)
+      expect(this.res.statusCode).to.equal(400)
+    })
+
+    it('should print a message when no content-type is included', function() {
+      this.HttpErrorHandler.badRequest(this.req, this.res)
+      expect(this.res.body).to.equal('client error')
+    })
+
+    it("should render a template including the error message when content-type is 'html'", function() {
+      this.req.accepts = () => 'html'
+      this.HttpErrorHandler.badRequest(this.req, this.res, 'an error')
+      expect(this.res.renderedTemplate).to.equal('general/400')
+      expect(this.res.renderedVariables).to.deep.equal({
+        title: 'Client Error',
+        message: 'an error'
+      })
+    })
+
+    it("should render a default template when content-type is 'html' and no message is provided", function() {
+      this.req.accepts = () => 'html'
+      this.HttpErrorHandler.badRequest(this.req, this.res)
+      expect(this.res.renderedTemplate).to.equal('general/400')
+      expect(this.res.renderedVariables).to.deep.equal({
+        title: 'Client Error',
+        message: undefined
+      })
+    })
+
+    it("should return a json object when content-type is 'json'", function() {
+      this.req.accepts = () => 'json'
+      this.HttpErrorHandler.badRequest(this.req, this.res, 'an error', {
+        foo: 'bar'
+      })
+      expect(JSON.parse(this.res.body)).to.deep.equal({
+        message: 'an error',
+        foo: 'bar'
+      })
+    })
+
+    it("should return an empty json object when content-type is 'json' and no message and info are provided", function() {
+      this.req.accepts = () => 'json'
+      this.HttpErrorHandler.badRequest(this.req, this.res)
+      expect(JSON.parse(this.res.body)).to.deep.equal({})
+    })
+  })
+
   describe('forbidden', function() {
     it('returns 403', function() {
       this.HttpErrorHandler.forbidden(this.req, this.res)