diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js index 07b4fffe4d..f432657e29 100644 --- a/services/web/app/src/Features/Errors/Errors.js +++ b/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 } diff --git a/services/web/app/src/Features/Errors/HttpErrorHandler.js b/services/web/app/src/Features/Errors/HttpErrorHandler.js index d6b5c76d2b..69d4655627 100644 --- a/services/web/app/src/Features/Errors/HttpErrorHandler.js +++ b/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'])) { diff --git a/services/web/test/unit/src/Errors/HttpErrorHandlerTests.js b/services/web/test/unit/src/Errors/HttpErrorHandlerTests.js index 88321f3a67..f83114eb37 100644 --- a/services/web/test/unit/src/Errors/HttpErrorHandlerTests.js +++ b/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)