feat(server): introduce user friendly server errors (#7111)

packages/backend/server/tests/auth/controller.spec.ts

@@ -119,7 +119,7 @@ test('should not be able to sign in if email is invalid', async t => {
     .send({ email: '' })
     .expect(400);
 
-  t.is(res.body.message, 'Invalid email address');
+  t.is(res.body.message, 'An invalid email provided.');
 });
 
 test('should not be able to sign in if forbidden', async t => {
@@ -130,7 +130,7 @@ test('should not be able to sign in if forbidden', async t => {
   await request(app.getHttpServer())
     .post('/api/auth/sign-in')
     .send({ email: u1.email })
-    .expect(HttpStatus.BAD_REQUEST);
+    .expect(HttpStatus.FORBIDDEN);
 
   t.true(mailer.sendSignInMail.notCalled);
 

packages/backend/server/tests/auth/guard.spec.ts

@@ -86,9 +86,11 @@ test('should not be able to visit private api if not signed in', async t => {
     .get('/private')
     .expect(HttpStatus.UNAUTHORIZED)
     .expect({
-      statusCode: 401,
-      message: 'You are not signed in.',
-      error: 'Unauthorized',
+      status: 401,
+      code: 'Unauthorized',
+      type: 'AUTHENTICATION_REQUIRED',
+      name: 'AUTHENTICATION_REQUIRED',
+      message: 'You must sign in first to access this resource.',
     });
 
   t.assert(true);

packages/backend/server/tests/auth/service.spec.ts

@@ -66,7 +66,7 @@ test('should throw if email duplicated', async t => {
   const { auth } = t.context;
 
   await t.throwsAsync(() => auth.signUp('u1', 'u1@affine.pro', '1'), {
-    message: 'Email was taken',
+    message: 'This email has already been registered.',
   });
 });
 
@@ -82,7 +82,7 @@ test('should throw if user not found', async t => {
   const { auth } = t.context;
 
   await t.throwsAsync(() => auth.signIn('u2@affine.pro', '1'), {
-    message: 'Invalid sign in credentials',
+    message: 'Wrong user email or password.',
   });
 });
 
@@ -95,7 +95,8 @@ test('should throw if password not set', async t => {
   });
 
   await t.throwsAsync(() => auth.signIn('u2@affine.pro', '1'), {
-    message: 'User Password is not set. Should login through email link.',
+    message:
+      'You are trying to sign in by a different method than you signed up with.',
   });
 });
 
@@ -103,7 +104,7 @@ test('should throw if password not match', async t => {
   const { auth } = t.context;
 
   await t.throwsAsync(() => auth.signIn('u1@affine.pro', '2'), {
-    message: 'Invalid sign in credentials',
+    message: 'Wrong user email or password.',
   });
 });
 
@@ -118,7 +119,7 @@ test('should be able to change password', async t => {
   await t.throwsAsync(
     () => auth.signIn('u1@affine.pro', '1' /* old password */),
     {
-      message: 'Invalid sign in credentials',
+      message: 'Wrong user email or password.',
     }
   );
 
@@ -135,7 +136,7 @@ test('should be able to change email', async t => {
   await auth.changeEmail(u1.id, 'u2@affine.pro');
 
   await t.throwsAsync(() => auth.signIn('u1@affine.pro' /* old email */, '1'), {
-    message: 'Invalid sign in credentials',
+    message: 'Wrong user email or password.',
   });
 
   signedInU1 = await auth.signIn('u2@affine.pro', '1');

packages/backend/server/tests/graphql.spec.ts

@@ -1,89 +0,0 @@
-import {
-  ForbiddenException,
-  HttpStatus,
-  INestApplication,
-} from '@nestjs/common';
-import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
-import testFn, { TestFn } from 'ava';
-import request from 'supertest';
-
-import { Public } from '../src/core/auth';
-import { createTestingApp } from './utils';
-
-@Public()
-@Resolver(() => String)
-class TestResolver {
-  greating = 'hello world';
-
-  @Query(() => String)
-  hello() {
-    return this.greating;
-  }
-
-  @Mutation(() => String)
-  update(@Args('greating') greating: string) {
-    this.greating = greating;
-    return this.greating;
-  }
-
-  @Query(() => String)
-  errorQuery() {
-    throw new ForbiddenException('forbidden query');
-  }
-
-  @Query(() => String)
-  unknownErrorQuery() {
-    throw new Error('unknown error');
-  }
-}
-
-const test = testFn as TestFn<{ app: INestApplication }>;
-
-function gql(app: INestApplication, query: string) {
-  return request(app.getHttpServer())
-    .post('/graphql')
-    .send({ query })
-    .expect(200);
-}
-
-test.beforeEach(async ctx => {
-  const { app } = await createTestingApp({
-    providers: [TestResolver],
-  });
-
-  ctx.context.app = app;
-});
-
-test.afterEach.always(async ctx => {
-  await ctx.context.app.close();
-});
-
-test('should be able to execute query', async t => {
-  const res = await gql(t.context.app, `query { hello }`);
-  t.is(res.body.data.hello, 'hello world');
-});
-
-test('should be able to execute mutation', async t => {
-  const res = await gql(t.context.app, `mutation { update(greating: "hi") }`);
-
-  t.is(res.body.data.update, 'hi');
-
-  const newRes = await gql(t.context.app, `query { hello }`);
-  t.is(newRes.body.data.hello, 'hi');
-});
-
-test('should be able to handle known http exception', async t => {
-  const res = await gql(t.context.app, `query { errorQuery }`);
-  const err = res.body.errors[0];
-  t.is(err.message, 'forbidden query');
-  t.is(err.extensions.code, HttpStatus.FORBIDDEN);
-  t.is(err.extensions.status, HttpStatus[HttpStatus.FORBIDDEN]);
-});
-
-test('should be able to handle unknown internal error', async t => {
-  const res = await gql(t.context.app, `query { unknownErrorQuery }`);
-  const err = res.body.errors[0];
-  t.is(err.message, 'Internal Server Error');
-  t.is(err.extensions.code, HttpStatus.INTERNAL_SERVER_ERROR);
-  t.is(err.extensions.status, HttpStatus[HttpStatus.INTERNAL_SERVER_ERROR]);
-});

packages/backend/server/tests/nestjs/error-handler.spec.ts

@@ -0,0 +1,199 @@
+import {
+  applyDecorators,
+  Controller,
+  Get,
+  HttpStatus,
+  INestApplication,
+  Logger,
+  LoggerService,
+} from '@nestjs/common';
+import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
+import {
+  SubscribeMessage as RawSubscribeMessage,
+  WebSocketGateway,
+} from '@nestjs/websockets';
+import testFn, { TestFn } from 'ava';
+import Sinon from 'sinon';
+import request from 'supertest';
+
+import { Public } from '../../src/core/auth';
+import {
+  AccessDenied,
+  GatewayErrorWrapper,
+  UserFriendlyError,
+} from '../../src/fundamentals';
+import { createTestingApp } from '../utils';
+
+@Public()
+@Resolver(() => String)
+class TestResolver {
+  greating = 'hello world';
+
+  @Query(() => String)
+  hello() {
+    return this.greating;
+  }
+
+  @Mutation(() => String)
+  update(@Args('greating') greating: string) {
+    this.greating = greating;
+    return this.greating;
+  }
+
+  @Query(() => String)
+  errorQuery() {
+    throw new AccessDenied();
+  }
+
+  @Query(() => String)
+  unknownErrorQuery() {
+    throw new Error('unknown error');
+  }
+}
+
+@Public()
+@Controller()
+class TestController {
+  @Get('/ok')
+  ok() {
+    return 'ok';
+  }
+
+  @Get('/throw-known-error')
+  throwKnownError() {
+    throw new AccessDenied();
+  }
+
+  @Get('/throw-unknown-error')
+  throwUnknownError() {
+    throw new Error('Unknown error');
+  }
+}
+
+const SubscribeMessage = (event: string) =>
+  applyDecorators(GatewayErrorWrapper(event), RawSubscribeMessage(event));
+
+@WebSocketGateway({ transports: ['websocket'], path: '/ws' })
+class TestGateway {
+  @SubscribeMessage('event:ok')
+  async ok() {
+    return {
+      data: 'ok',
+    };
+  }
+
+  @SubscribeMessage('event:throw-known-error')
+  async throwKnownError() {
+    throw new AccessDenied();
+  }
+
+  @SubscribeMessage('event:throw-unknown-error')
+  async throwUnknownError() {
+    throw new Error('Unknown error');
+  }
+}
+
+const test = testFn as TestFn<{
+  app: INestApplication;
+  logger: Sinon.SinonStubbedInstance<LoggerService>;
+}>;
+
+function gql(app: INestApplication, query: string) {
+  return request(app.getHttpServer())
+    .post('/graphql')
+    .send({ query })
+    .expect(200);
+}
+
+test.beforeEach(async ({ context }) => {
+  const { app } = await createTestingApp({
+    providers: [TestResolver, TestGateway],
+    controllers: [TestController],
+  });
+
+  context.logger = Sinon.stub(new Logger().localInstance);
+
+  context.app = app;
+});
+
+test.afterEach.always(async ctx => {
+  await ctx.context.app.close();
+});
+
+test('should be able to execute query', async t => {
+  const res = await gql(t.context.app, `query { hello }`);
+  t.is(res.body.data.hello, 'hello world');
+});
+
+test('should be able to handle known user error in graphql query', async t => {
+  const res = await gql(t.context.app, `query { errorQuery }`);
+  const err = res.body.errors[0];
+  t.is(err.message, 'You do not have permission to access this resource.');
+  t.is(err.extensions.status, HttpStatus.FORBIDDEN);
+  t.is(err.extensions.name, 'ACCESS_DENIED');
+  t.true(t.context.logger.error.notCalled);
+});
+
+test('should be able to handle unknown internal error in graphql query', async t => {
+  const res = await gql(t.context.app, `query { unknownErrorQuery }`);
+  const err = res.body.errors[0];
+  t.is(err.message, 'An internal error occurred.');
+  t.is(err.extensions.status, HttpStatus.INTERNAL_SERVER_ERROR);
+  t.is(err.extensions.name, 'INTERNAL_SERVER_ERROR');
+  t.true(t.context.logger.error.calledOnceWith('Internal server error'));
+});
+
+test('should be able to respond request', async t => {
+  const res = await request(t.context.app.getHttpServer())
+    .get('/ok')
+    .expect(200);
+  t.is(res.text, 'ok');
+});
+
+test('should be able to handle known user error in http request', async t => {
+  const res = await request(t.context.app.getHttpServer())
+    .get('/throw-known-error')
+    .expect(HttpStatus.FORBIDDEN);
+  t.is(res.body.message, 'You do not have permission to access this resource.');
+  t.is(res.body.name, 'ACCESS_DENIED');
+  t.true(t.context.logger.error.notCalled);
+});
+
+test('should be able to handle unknown internal error in http request', async t => {
+  const res = await request(t.context.app.getHttpServer())
+    .get('/throw-unknown-error')
+    .expect(HttpStatus.INTERNAL_SERVER_ERROR);
+  t.is(res.body.message, 'An internal error occurred.');
+  t.is(res.body.name, 'INTERNAL_SERVER_ERROR');
+  t.true(t.context.logger.error.calledOnceWith('Internal server error'));
+});
+
+// Hard to test through websocket, will call event handler directly
+test('should be able to response websocket event', async t => {
+  const gateway = t.context.app.get(TestGateway);
+
+  const res = await gateway.ok();
+  t.is(res.data, 'ok');
+});
+
+test('should be able to handle known user error in websocket event', async t => {
+  const gateway = t.context.app.get(TestGateway);
+
+  const { error } = (await gateway.throwKnownError()) as unknown as {
+    error: UserFriendlyError;
+  };
+  t.is(error.message, 'You do not have permission to access this resource.');
+  t.is(error.name, 'ACCESS_DENIED');
+  t.true(t.context.logger.error.notCalled);
+});
+
+test('should be able to handle unknown internal error in websocket event', async t => {
+  const gateway = t.context.app.get(TestGateway);
+
+  const { error } = (await gateway.throwUnknownError()) as unknown as {
+    error: UserFriendlyError;
+  };
+  t.is(error.message, 'An internal error occurred.');
+  t.is(error.name, 'INTERNAL_SERVER_ERROR');
+  t.true(t.context.logger.error.calledOnceWith('Internal server error'));
+});

packages/backend/server/tests/oauth/controller.spec.ts

@@ -86,12 +86,15 @@ test('should throw if provider is invalid', async t => {
     .get('/oauth/login?provider=Invalid')
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'Invalid OAuth provider',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'INVALID_INPUT',
+      name: 'UNKNOWN_OAUTH_PROVIDER',
+      message: 'Unknown authentication provider Invalid.',
+      data: { name: 'Invalid' },
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 test('should be able to save oauth state', async t => {
@@ -124,12 +127,15 @@ test('should throw if code is missing in callback uri', async t => {
     .get('/oauth/callback')
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'Missing query parameter `code`',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'BAD_REQUEST',
+      name: 'MISSING_OAUTH_QUERY_PARAMETER',
+      message: 'Missing query parameter `code`.',
+      data: { name: 'code' },
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 test('should throw if state is missing in callback uri', async t => {
@@ -139,27 +145,50 @@ test('should throw if state is missing in callback uri', async t => {
     .get('/oauth/callback?code=1')
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'Invalid callback state parameter',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'BAD_REQUEST',
+      name: 'MISSING_OAUTH_QUERY_PARAMETER',
+      message: 'Missing query parameter `state`.',
+      data: { name: 'state' },
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 test('should throw if state is expired', async t => {
+  const { app, oauth } = t.context;
+  Sinon.stub(oauth, 'isValidState').resolves(true);
+
+  await request(app.getHttpServer())
+    .get('/oauth/callback?code=1&state=1')
+    .expect(HttpStatus.BAD_REQUEST)
+    .expect({
+      status: 400,
+      code: 'Bad Request',
+      type: 'BAD_REQUEST',
+      name: 'OAUTH_STATE_EXPIRED',
+      message: 'OAuth state expired, please try again.',
+    });
+
+  t.pass();
+});
+
+test('should throw if state is invalid', async t => {
   const { app } = t.context;
 
   await request(app.getHttpServer())
     .get('/oauth/callback?code=1&state=1')
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'OAuth state expired, please try again.',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'BAD_REQUEST',
+      name: 'INVALID_OAUTH_CALLBACK_STATE',
+      message: 'Invalid callback state parameter.',
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 test('should throw if provider is missing in state', async t => {
@@ -167,17 +196,21 @@ test('should throw if provider is missing in state', async t => {
 
   // @ts-expect-error mock
   Sinon.stub(oauth, 'getOAuthState').resolves({});
+  Sinon.stub(oauth, 'isValidState').resolves(true);
 
   await request(app.getHttpServer())
     .get(`/oauth/callback?code=1&state=1`)
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'Missing callback state parameter `provider`',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'BAD_REQUEST',
+      name: 'MISSING_OAUTH_QUERY_PARAMETER',
+      message: 'Missing query parameter `provider`.',
+      data: { name: 'provider' },
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 test('should throw if provider is invalid in callback uri', async t => {
@@ -185,23 +218,28 @@ test('should throw if provider is invalid in callback uri', async t => {
 
   // @ts-expect-error mock
   Sinon.stub(oauth, 'getOAuthState').resolves({ provider: 'Invalid' });
+  Sinon.stub(oauth, 'isValidState').resolves(true);
 
   await request(app.getHttpServer())
     .get(`/oauth/callback?code=1&state=1`)
     .expect(HttpStatus.BAD_REQUEST)
     .expect({
-      statusCode: 400,
-      message: 'Invalid provider',
-      error: 'Bad Request',
+      status: 400,
+      code: 'Bad Request',
+      type: 'INVALID_INPUT',
+      name: 'UNKNOWN_OAUTH_PROVIDER',
+      message: 'Unknown authentication provider Invalid.',
+      data: { name: 'Invalid' },
     });
 
-  t.assert(true);
+  t.pass();
 });
 
 function mockOAuthProvider(app: INestApplication, email: string) {
   const provider = app.get(GoogleOAuthProvider);
   const oauth = app.get(OAuthService);
 
+  Sinon.stub(oauth, 'isValidState').resolves(true);
   Sinon.stub(oauth, 'getOAuthState').resolves({
     provider: OAuthProviderName.Google,
     redirectUri: '/',
@@ -259,7 +297,7 @@ test('should throw if account register in another way', async t => {
   t.is(link.pathname, '/signIn');
   t.is(
     link.searchParams.get('error'),
-    'The account with provided email is not register in the same way.'
+    'You are trying to sign in by a different method than you signed up with.'
   );
 });
 

packages/backend/server/tests/payment/service.spec.ts

@@ -356,7 +356,7 @@ test('should throw if user has subscription already', async t => {
         redirectUrl: '',
         idempotencyKey: '',
       }),
-    { message: "You've already subscribed to the pro plan" }
+    { message: 'You have already subscribed to the pro plan.' }
   );
 });
 

packages/backend/server/tests/utils/utils.ts

@@ -10,6 +10,7 @@ import type { Response } from 'supertest';
 import { AppModule, FunctionalityModules } from '../../src/app.module';
 import { AuthGuard, AuthModule } from '../../src/core/auth';
 import { UserFeaturesInit1698652531198 } from '../../src/data/migrations/1698652531198-user-features-init';
+import { GlobalExceptionFilter } from '../../src/fundamentals';
 import { GqlModule } from '../../src/fundamentals/graphql';
 
 async function flushDB(client: PrismaClient) {
@@ -116,6 +117,7 @@ export async function createTestingApp(moduleDef: TestingModuleMeatdata = {}) {
     logger: ['warn'],
   });
 
+  app.useGlobalFilters(new GlobalExceptionFilter(app.getHttpAdapter()));
   app.use(
     graphqlUploadExpress({
       maxFileSize: 10 * 1024 * 1024,

packages/backend/server/tests/workspace.e2e.ts

@@ -9,7 +9,6 @@ import {
   acceptInviteById,
   createTestingApp,
   createWorkspace,
-  currentUser,
   getWorkspacePublicPages,
   inviteUser,
   publishPage,
@@ -43,19 +42,6 @@ test('should register a user', async t => {
   t.is(user.email, 'u1@affine.pro', 'user.email is not valid');
 });
 
-test.skip('should be throttled at call signUp', async t => {
-  const { app } = t.context;
-  let token = '';
-  for (let i = 0; i < 10; i++) {
-    token = (await signUp(app, `u${i}`, `u${i}@affine.pro`, `${i}`)).token
-      .token;
-    // throttles are applied to each endpoint separately
-    await currentUser(app, token);
-  }
-  await t.throwsAsync(() => signUp(app, 'u11', 'u11@affine.pro', '11'));
-  await t.throwsAsync(() => currentUser(app, token));
-});
-
 test('should create a workspace', async t => {
   const { app } = t.context;
   const user = await signUp(app, 'u1', 'u1@affine.pro', '1');
@@ -128,14 +114,22 @@ test('should share a page', async t => {
   t.is(resp4.statusCode, 404, 'should not get shared doc without token');
 
   const msg1 = await publishPage(app, u2.token.token, 'not_exists_ws', 'page2');
-  t.is(msg1, 'Permission denied', 'unauthorized user can share page');
+  t.is(
+    msg1,
+    'You do not have permission to access workspace not_exists_ws.',
+    'unauthorized user can share page'
+  );
   const msg2 = await revokePublicPage(
     app,
     u2.token.token,
     'not_exists_ws',
     'page2'
   );
-  t.is(msg2, 'Permission denied', 'unauthorized user can share page');
+  t.is(
+    msg2,
+    'You do not have permission to access workspace not_exists_ws.',
+    'unauthorized user can share page'
+  );
 
   await acceptInviteById(
     app,