Merge pull request #33413 from overleaf/revert-33040-em-parse-req-errors

Revert "Replace isZodErrorLike with custom error types in request validation"

GitOrigin-RevId: 1f51fe9e14ffabf283f1229552d3887136420f8f

services/notifications/app.ts

@@ -13,10 +13,8 @@ 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 {
-  InvalidParamsError,
-  InvalidRequestError,
-} from '@overleaf/validation-tools'
+import { isZodErrorLike } from 'zod-validation-error'
+import { ParamsError } from '@overleaf/validation-tools'
 
 const app = express()
 
@@ -58,10 +56,10 @@ const handleApiError: ErrorRequestHandler = (
   next: NextFunction
 ) => {
   req.logger.addFields({ err })
-  if (err instanceof InvalidParamsError) {
+  if (err instanceof ParamsError) {
     req.logger.setLevel('warn')
     res.sendStatus(404)
-  } else if (err instanceof InvalidRequestError) {
+  } else if (isZodErrorLike(err)) {
     req.logger.setLevel('warn')
     res.sendStatus(400)
   } else {

services/real-time/app/js/Router.js

@@ -40,7 +40,7 @@ export default Router = {
     attrs.client_id = client.id
     attrs.err = error
     attrs.method = method
-    if (attrs.validation && isZodErrorLike(error)) {
+    if (isZodErrorLike(error)) {
       logger.info(attrs, 'validation error')
       let message = 'invalid'
       try {
@@ -456,7 +456,6 @@ export default Router = {
           joinDocSchema.parse({ doc_id: docId, fromVersion, options })
         } catch (error) {
           return Router._handleError(callback, error, client, 'joinDoc', {
-            validation: 1,
             disconnect: 1,
           })
         }
@@ -486,7 +485,6 @@ export default Router = {
           zz.objectId().parse(docId)
         } catch (error) {
           return Router._handleError(callback, error, client, 'joinDoc', {
-            validation: 1,
             disconnect: 1,
           })
         }
@@ -572,7 +570,6 @@ 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

@@ -1,14 +1,11 @@
-import { fromZodError } from 'zod-validation-error'
-import {
-  InvalidRequestError,
-  InvalidParamsError,
-} from '@overleaf/validation-tools'
+import { isZodErrorLike, fromZodError } from 'zod-validation-error'
 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)
@@ -45,7 +42,7 @@ async function handleError(error, req, res, next) {
     if (shouldSendErrorResponse) {
       notFound(req, res)
     }
-  } else if (error instanceof InvalidParamsError) {
+  } else if (error instanceof ParamsError) {
     req.logger.setLevel('warn')
     if (shouldSendErrorResponse) {
       notFound(req, res)
@@ -107,11 +104,11 @@ async function handleError(error, req, res, next) {
       res.status(400)
       plainTextResponse(res, error.message)
     }
-  } else if (error instanceof InvalidRequestError) {
+  } else if (isZodErrorLike(error)) {
     req.logger.setLevel('warn')
     res.status(400)
     if (shouldSendErrorResponse) {
-      const validationError = fromZodError(error.zodError)
+      const validationError = fromZodError(error)
       res.render('general/400', { message: validationError.message })
     }
   } else {
@@ -129,10 +126,7 @@ async function handleError(error, req, res, next) {
 
 function handleApiError(err, req, res, next) {
   req.logger.addFields({ err })
-  if (
-    err instanceof Errors.NotFoundError ||
-    err instanceof InvalidParamsError
-  ) {
+  if (err instanceof Errors.NotFoundError || err instanceof ParamsError) {
     req.logger.setLevel('warn')
     res.sendStatus(404)
   } else if (
@@ -147,7 +141,7 @@ function handleApiError(err, req, res, next) {
   } else if (err instanceof Errors.ForbiddenError) {
     req.logger.setLevel('warn')
     res.sendStatus(403)
-  } else if (err instanceof InvalidRequestError) {
+  } else if (isZodErrorLike(err)) {
     req.logger.setLevel('warn')
     res.sendStatus(400)
   } else {

services/web/app/src/Features/Security/LoginRateLimiter.mjs

@@ -2,7 +2,6 @@ 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',
@@ -13,11 +12,7 @@ const rateLimiterLoginEmail = new RateLimiter(
 )
 
 async function processLoginRequest(email) {
-  try {
-    email = EmailHelper.emailSchema.parse(email)
-  } catch (err) {
-    throw new InvalidRequestError(err)
-  }
+  email = EmailHelper.emailSchema.parse(email)
   try {
     await rateLimiterLoginEmail.consume(email, 1, {
       method: 'email',

services/web/app/src/infrastructure/Validation.mjs

@@ -2,7 +2,7 @@
 
 import { parseReq, z, zz } from '@overleaf/validation-tools'
 
-export { InvalidParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
+export { ParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
 
 export default {
   parseReq,

services/web/test/acceptance/src/AuthenticationTests.mjs

@@ -176,12 +176,31 @@ describe('Authentication', function () {
       }
     })
     it('should return 400 with bad email (missing @)', async function () {
-      const { statusCode } = await tryLoginWithEmail('foo')
+      const { statusCode, body } = 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 } = await tryLoginWithEmail(1)
+      const { statusCode, body } = 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

@@ -219,13 +219,14 @@ 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 } = await this.user.doRequest('POST', {
+      const { response, body } = 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

@@ -375,6 +375,11 @@ 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()
             }
           )
@@ -399,6 +404,14 @@ 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/Subscription/SubscriptionController.test.mjs

@@ -685,7 +685,7 @@ describe('SubscriptionController', function () {
       ctx.next = sinon.stub()
       await expect(
         ctx.SubscriptionController.pauseSubscription(ctx.req, ctx.res, ctx.next)
-      ).to.be.rejectedWith('Invalid request parameters')
+      ).to.be.rejectedWith('Invalid params')
     })
 
     it('should throw an error if an invalid pause length is provided', async function (ctx) {

services/web/test/unit/src/Tags/TagsController.test.mjs

@@ -1,6 +1,6 @@
 import { assert, beforeEach, describe, it, vi } from 'vitest'
 import sinon from 'sinon'
-import { InvalidRequestError } from '@overleaf/validation-tools'
+import { ZodError } from 'zod'
 
 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(
-        InvalidRequestError
+        ZodError
       )
     })
   })