alois/Verso
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #33490 from overleaf/em-parse-req-errors-2

Browse Source 
Reintroduce custom error types in request validation

GitOrigin-RevId: 1985ca04c8fe693fb836b042517d94700343bc46
This commit is contained in:
Eric Mc Sween
2026-05-07 13:32:29 -04:00
committed by Copybot
parent 8c0589df7f
commit 2bb35fafb8
17 changed files with 105 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
libraries/validation-tools/Errors.js
+26 -2
View File
@@ -1,5 +1,29 @@
// @ts-check
const OError = require('@overleaf/o-error')
class ParamsError extends OError {}
/**
* @typedef {import('zod').ZodError} ZodError
*/
module.exports = { ParamsError }
class InvalidRequestError extends OError {
/**
* @param {ZodError} zodError
*/
constructor(zodError) {
super('Invalid request', {}, zodError)
this.zodError = zodError
}
}
class InvalidParamsError extends OError {
/**
* @param {ZodError} zodError
*/
constructor(zodError) {
super('Invalid request parameters', {}, zodError)
this.zodError = zodError
}
}
module.exports = { InvalidParamsError, InvalidRequestError }
libraries/validation-tools/handleValidationError.js
+15 -10
View File
@@ -1,15 +1,20 @@
const { isZodErrorLike, fromError } = require('zod-validation-error')
const { fromError } = require('zod-validation-error')
const { InvalidParamsError, InvalidRequestError } = require('./Errors')
function createHandleValidationError(statusCode = 400) {
return [
(err, req, res, next) => {
if (!isZodErrorLike(err)) {
return next(err)
}
res.status(statusCode).json({ ...fromError(err), statusCode })
},
]
return (err, req, res, next) => {
if (err instanceof InvalidParamsError) {
res
.status(404)
.json({ error: fromError(err.zodError).toString(), statusCode: 404 })
} else if (err instanceof InvalidRequestError) {
res
.status(statusCode)
.json({ error: fromError(err.zodError).toString(), statusCode })
} else {
next(err)
}
}
}
const handleValidationError = createHandleValidationError(400)
libraries/validation-tools/index.js
+3 -2
View File
@@ -1,4 +1,4 @@
const { ParamsError } = require('./Errors')
const { InvalidParamsError, InvalidRequestError } = require('./Errors')
const { z } = require('zod')
const { zz } = require('./zodHelpers')
const { parseReq } = require('./parseReq')
@@ -15,5 +15,6 @@ module.exports = {
parseReq,
handleValidationError,
createHandleValidationError,
ParamsError,
InvalidRequestError,
InvalidParamsError,
}
libraries/validation-tools/parseReq.js
+3 -3
View File
@@ -1,5 +1,5 @@
// @ts-check
const { ParamsError } = require('./Errors')
const { InvalidRequestError, InvalidParamsError } = require('./Errors')
/**
* @typedef {import('zod').ZodType} ZodType
@@ -25,9 +25,9 @@ function parseReq(req, schema) {
return parsed.data
} else if (parsed.error.issues.some(issue => issue.path[0] === 'params')) {
// Parts of the URL path failed to validate; throw a specific error
throw new ParamsError('Invalid params').withCause(parsed.error)
throw new InvalidParamsError(parsed.error)
} else {
throw parsed.error
throw new InvalidRequestError(parsed.error)
}
}
libraries/validation-tools/test/unit/src/validateReq.test.ts
+6 -4
View File
@@ -54,7 +54,7 @@ describe('parseReq', () => {
}),
})
)
).toThrowError(expect.objectContaining({ name: 'ParamsError' }))
).toThrowError(expect.objectContaining({ name: 'InvalidParamsError' }))
})
it('should throw an error containing issues if the schema is invalid', () => {
@@ -75,9 +75,11 @@ describe('parseReq', () => {
)
).toThrowError(
expect.objectContaining({
issues: expect.arrayContaining([
expect.objectContaining({ path: ['body', 'name'] }),
]),
zodError: expect.objectContaining({
issues: expect.arrayContaining([
expect.objectContaining({ path: ['body', 'name'] }),
]),
}),
})
)
})
server-ce/test/host-admin.js
+11 -6
View File
@@ -5,8 +5,12 @@ import { execFile as execFileCb } from 'node:child_process'
import bodyParser from 'body-parser'
import express from 'express'
import YAML from 'js-yaml'
import { isZodErrorLike } from 'zod-validation-error'
import { ParamsError, parseReq, z } from '@overleaf/validation-tools'
import {
InvalidParamsError,
InvalidRequestError,
parseReq,
z,
} from '@overleaf/validation-tools'
import { expressify } from '@overleaf/promise-utils'
const execFile = promisify(execFileCb)
@@ -474,12 +478,13 @@ app.delete(
)
app.use((error, req, res, next) => {
if (error instanceof ParamsError) {
if (error instanceof InvalidParamsError) {
res.status(404).json({ error })
} else if (isZodErrorLike(error)) {
res.status(400).json({ error })
} else if (error instanceof InvalidRequestError) {
res.status(400).json({ error: error.zodError })
} else {
next(error)
}
next(error)
})
purgeDataDir()
services/notifications/app.ts
+6 -4
View File
@@ -13,8 +13,10 @@ import methodOverride from 'method-override'
import { mongoClient } from './app/js/mongodb.js'
import NotificationsController from './app/js/NotificationsController.ts'
import HealthCheckController from './app/js/HealthCheckController.ts'
import { isZodErrorLike } from 'zod-validation-error'
import { ParamsError } from '@overleaf/validation-tools'
import {
InvalidParamsError,
InvalidRequestError,
} from '@overleaf/validation-tools'
const app = express()
@@ -56,10 +58,10 @@ const handleApiError: ErrorRequestHandler = (
next: NextFunction
) => {
req.logger.addFields({ err })
if (err instanceof ParamsError) {
if (err instanceof InvalidParamsError) {
req.logger.setLevel('warn')
res.sendStatus(404)
} else if (isZodErrorLike(err)) {
} else if (err instanceof InvalidRequestError) {
req.logger.setLevel('warn')
res.sendStatus(400)
} else {
services/real-time/app/js/Router.js
+5 -2
View File
@@ -40,7 +40,7 @@ export default Router = {
attrs.client_id = client.id
attrs.err = error
attrs.method = method
if (isZodErrorLike(error)) {
if (attrs.validation && isZodErrorLike(error)) {
logger.info(attrs, 'validation error')
let message = 'invalid'
try {
@@ -456,6 +456,7 @@ export default Router = {
joinDocSchema.parse({ doc_id: docId, fromVersion, options })
} catch (error) {
return Router._handleError(callback, error, client, 'joinDoc', {
validation: 1,
disconnect: 1,
})
}
@@ -484,7 +485,8 @@ export default Router = {
try {
zz.objectId().parse(docId)
} catch (error) {
return Router._handleError(callback, error, client, 'joinDoc', {
return Router._handleError(callback, error, client, 'leaveDoc', {
validation: 1,
disconnect: 1,
})
}
@@ -570,6 +572,7 @@ export default Router = {
applyOtUpdateSchema.parse({ doc_id: docId, update })
} catch (error) {
return Router._handleError(callback, error, client, 'applyOtUpdate', {
validation: 1,
disconnect: 1,
})
}
services/web/app/src/Features/Errors/ErrorController.mjs
+13 -7
View File
@@ -1,11 +1,14 @@
import { isZodErrorLike, fromZodError } from 'zod-validation-error'
import { fromZodError } from 'zod-validation-error'
import {
InvalidRequestError,
InvalidParamsError,
} from '@overleaf/validation-tools'
import Errors, { NotFoundError } from './Errors.js'
import SessionManager from '../Authentication/SessionManager.mjs'
import SamlLogHandler from '../SamlLog/SamlLogHandler.mjs'
import HttpErrorHandler from './HttpErrorHandler.mjs'
import { plainTextResponse } from '../../infrastructure/Response.mjs'
import { expressifyErrorHandler } from '@overleaf/promise-utils'
import { ParamsError } from '@overleaf/validation-tools'
function notFound(req, res) {
res.status(404)
@@ -42,7 +45,7 @@ async function handleError(error, req, res, next) {
if (shouldSendErrorResponse) {
notFound(req, res)
}
} else if (error instanceof ParamsError) {
} else if (error instanceof InvalidParamsError) {
req.logger.setLevel('warn')
if (shouldSendErrorResponse) {
notFound(req, res)
@@ -104,11 +107,11 @@ async function handleError(error, req, res, next) {
res.status(400)
plainTextResponse(res, error.message)
}
} else if (isZodErrorLike(error)) {
} else if (error instanceof InvalidRequestError) {
req.logger.setLevel('warn')
res.status(400)
if (shouldSendErrorResponse) {
const validationError = fromZodError(error)
const validationError = fromZodError(error.zodError)
res.render('general/400', { message: validationError.message })
}
} else {
@@ -126,7 +129,10 @@ async function handleError(error, req, res, next) {
function handleApiError(err, req, res, next) {
req.logger.addFields({ err })
if (err instanceof Errors.NotFoundError || err instanceof ParamsError) {
if (
err instanceof Errors.NotFoundError ||
err instanceof InvalidParamsError
) {
req.logger.setLevel('warn')
res.sendStatus(404)
} else if (
@@ -141,7 +147,7 @@ function handleApiError(err, req, res, next) {
} else if (err instanceof Errors.ForbiddenError) {
req.logger.setLevel('warn')
res.sendStatus(403)
} else if (isZodErrorLike(err)) {
} else if (err instanceof InvalidRequestError) {
req.logger.setLevel('warn')
res.sendStatus(400)
} else {
services/web/app/src/Features/Security/LoginRateLimiter.mjs
+6 -1
View File
@@ -2,6 +2,7 @@ import { RateLimiter } from '../../infrastructure/RateLimiter.mjs'
import { callbackify } from '@overleaf/promise-utils'
import Settings from '@overleaf/settings'
import EmailHelper from '../Helpers/EmailHelper.mjs'
import { InvalidRequestError } from '@overleaf/validation-tools'
const rateLimiterLoginEmail = new RateLimiter(
'login',
@@ -12,7 +13,11 @@ const rateLimiterLoginEmail = new RateLimiter(
)
async function processLoginRequest(email) {
email = EmailHelper.emailSchema.parse(email)
try {
email = EmailHelper.emailSchema.parse(email)
} catch (err) {
throw new InvalidRequestError(err)
}
try {
await rateLimiterLoginEmail.consume(email, 1, {
method: 'email',
services/web/app/src/infrastructure/Validation.mjs
+1 -1
View File
@@ -2,7 +2,7 @@
import { parseReq, z, zz } from '@overleaf/validation-tools'
export { ParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
export { InvalidParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
export default {
parseReq,
services/web/test/acceptance/src/AuthenticationTests.mjs
+2 -21
View File
@@ -176,31 +176,12 @@ describe('Authentication', function () {
}
})
it('should return 400 with bad email (missing @)', async function () {
const { statusCode, body } = await tryLoginWithEmail('foo')
const { statusCode } = await tryLoginWithEmail('foo')
expect(statusCode).to.equal(400)
expect(body).to.deep.equal({
name: 'ZodValidationError',
details: [
{ code: 'custom', path: [], message: 'Invalid email address' },
],
statusCode: 400,
})
})
it('should return 400 with bad email (number)', async function () {
const { statusCode, body } = await tryLoginWithEmail(1)
const { statusCode } = await tryLoginWithEmail(1)
expect(statusCode).to.equal(400)
expect(body).to.deep.equal({
name: 'ZodValidationError',
details: [
{
expected: 'string',
code: 'invalid_type',
path: [],
message: 'Invalid input: expected string, received number',
},
],
statusCode: 400,
})
})
})
})
services/web/test/acceptance/src/ProjectCRUDTests.mjs
+1 -2
View File
@@ -219,14 +219,13 @@ describe('Project CRUD', function () {
})
it('returns a 400 when publicAccessLevel is an unsupported access level', async function () {
await this.user.makePrivate(this.projectId)
const { response, body } = await this.user.doRequest('POST', {
const { response } = await this.user.doRequest('POST', {
url: `/project/${this.projectId}/settings/admin`,
json: {
publicAccessLevel: 'readOnly',
},
})
expect(response.statusCode).to.equal(400)
expect(body.details[0].message).to.equal('unexpected access level')
const project = await Project.findById(this.projectId).exec()
expect(project.publicAccesLevel).to.equal('private')
})
services/web/test/acceptance/src/ProjectInviteTests.mjs
-13
View File
@@ -375,11 +375,6 @@ describe('ProjectInviteTests', function () {
return done(err)
}
expect(response.statusCode).to.equal(400)
expect(body.details).to.have.lengthOf(1)
expect(response.body.details[0].path).to.eql(['body', 'email'])
expect(response.body.details[0].message).to.equal(
'Invalid input: expected string, received object'
)
done()
}
)
@@ -404,14 +399,6 @@ describe('ProjectInviteTests', function () {
return done(err)
}
expect(response.statusCode).to.equal(400)
expect(body.details).to.have.lengthOf(1)
expect(response.body.details[0].path).to.eql([
'body',
'privileges',
])
expect(response.body.details[0].message).to.equal(
'Invalid option: expected one of "readOnly"|"readAndWrite"|"review"'
)
done()
}
)
services/web/test/unit/src/Compile/CompileController.test.mjs
+4 -4
View File
@@ -845,8 +845,8 @@ describe('CompileController', function () {
it('should reject the request', function (ctx) {
ctx.next.should.have.been.calledWithMatch({
name: 'ParamsError',
cause: asZodError({
name: 'InvalidParamsError',
zodError: asZodError({
code: 'custom',
path: ['params', 'file'],
message: 'path traversal detected',
@@ -868,8 +868,8 @@ describe('CompileController', function () {
it('should reject the request', function (ctx) {
ctx.next.should.have.been.calledWithMatch({
name: 'ParamsError',
cause: asZodError({
name: 'InvalidParamsError',
zodError: asZodError({
origin: 'string',
code: 'invalid_format',
format: 'regex',
services/web/test/unit/src/Subscription/SubscriptionController.test.mjs
+1 -1
View File
@@ -695,7 +695,7 @@ describe('SubscriptionController', function () {
ctx.next = sinon.stub()
await expect(
ctx.SubscriptionController.pauseSubscription(ctx.req, ctx.res, ctx.next)
).to.be.rejectedWith('Invalid params')
).to.be.rejectedWith('Invalid request parameters')
})
it('should throw an error if an invalid pause length is provided', async function (ctx) {
services/web/test/unit/src/Tags/TagsController.test.mjs
+2 -2
View File
@@ -1,6 +1,6 @@
import { assert, beforeEach, describe, it, vi } from 'vitest'
import sinon from 'sinon'
import { ZodError } from 'zod'
import { InvalidRequestError } from '@overleaf/validation-tools'
const modulePath = '../../../../app/src/Features/Tags/TagsController.mjs'
@@ -202,7 +202,7 @@ describe('TagsController', function () {
it('without a name', function (ctx) {
ctx.req.body = { name: undefined }
ctx.TagsController.renameTag(ctx.req, ctx.res).should.be.rejectedWith(
ZodError
InvalidRequestError
)
})
})