From eb642318994df9a2e260c00a3b29ec5470cbb847 Mon Sep 17 00:00:00 2001 From: darkskygit Date: Tue, 3 Dec 2024 06:47:16 +0000 Subject: [PATCH] feat(server): improve logs (#8977) --- packages/backend/server/src/core/auth/controller.ts | 8 ++++---- packages/backend/server/src/core/utils/validators.ts | 2 +- packages/backend/server/src/fundamentals/error/def.ts | 10 ++++++++-- .../server/src/fundamentals/error/errors.gen.ts | 10 +++++++--- .../server/src/fundamentals/graphql/logger-plugin.ts | 9 ++++++--- .../server/src/fundamentals/nestjs/exception.ts | 4 +++- packages/backend/server/src/schema.gql | 6 +++++- packages/backend/server/tests/auth/controller.spec.ts | 2 +- 8 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/backend/server/src/core/auth/controller.ts b/packages/backend/server/src/core/auth/controller.ts index 9258ad2cf2..5601ad4e24 100644 --- a/packages/backend/server/src/core/auth/controller.ts +++ b/packages/backend/server/src/core/auth/controller.ts @@ -75,7 +75,7 @@ export class AuthController { @Body() params?: { email: string } ): Promise { if (!params?.email) { - throw new InvalidEmail(); + throw new InvalidEmail({ email: 'not provided' }); } validators.assertValidEmail(params.email); @@ -171,7 +171,7 @@ export class AuthController { // verify domain has MX, SPF, DMARC records const [name, domain, ...rest] = email.split('@'); if (rest.length || !domain) { - throw new InvalidEmail(); + throw new InvalidEmail({ email }); } const [mx, spf, dmarc] = await Promise.allSettled([ resolveMx(domain).then(t => t.map(mx => mx.exchange).filter(Boolean)), @@ -183,11 +183,11 @@ export class AuthController { ), ]).then(t => t.filter(t => t.status === 'fulfilled').map(t => t.value)); if (!mx?.length || !spf?.length || !dmarc?.length) { - throw new InvalidEmail(); + throw new InvalidEmail({ email }); } // filter out alias emails if (name.includes('+') || name.includes('.')) { - throw new InvalidEmail(); + throw new InvalidEmail({ email }); } } } diff --git a/packages/backend/server/src/core/utils/validators.ts b/packages/backend/server/src/core/utils/validators.ts index e6d3aeeb36..f470a62717 100644 --- a/packages/backend/server/src/core/utils/validators.ts +++ b/packages/backend/server/src/core/utils/validators.ts @@ -5,7 +5,7 @@ import { InvalidEmail, InvalidPasswordLength } from '../../fundamentals'; export function assertValidEmail(email: string) { const result = z.string().email().safeParse(email); if (!result.success) { - throw new InvalidEmail(); + throw new InvalidEmail({ email }); } } diff --git a/packages/backend/server/src/fundamentals/error/def.ts b/packages/backend/server/src/fundamentals/error/def.ts index 7d92a1d7e9..10403ee477 100644 --- a/packages/backend/server/src/fundamentals/error/def.ts +++ b/packages/backend/server/src/fundamentals/error/def.ts @@ -101,7 +101,12 @@ export class UserFriendlyError extends Error { log(context: string) { // ignore all user behavior error log if (this.type !== 'internal_server_error') { - return; + // always record auth related error + const isAuthError = + typeof this.stack === 'string' && + (this.stack.includes('/core/auth/') || + this.stack.includes('/plugins/oauth/')); + if (!isAuthError) return; } new Logger(context).error( @@ -261,7 +266,8 @@ export const USER_FRIENDLY_ERRORS = { }, invalid_email: { type: 'invalid_input', - message: 'An invalid email provided.', + args: { email: 'string' }, + message: ({ email }) => `An invalid email provided: ${email}`, }, invalid_password_length: { type: 'invalid_input', diff --git a/packages/backend/server/src/fundamentals/error/errors.gen.ts b/packages/backend/server/src/fundamentals/error/errors.gen.ts index dc9effcfff..ed05069704 100644 --- a/packages/backend/server/src/fundamentals/error/errors.gen.ts +++ b/packages/backend/server/src/fundamentals/error/errors.gen.ts @@ -89,10 +89,14 @@ export class OauthAccountAlreadyConnected extends UserFriendlyError { super('bad_request', 'oauth_account_already_connected', message); } } +@ObjectType() +class InvalidEmailDataType { + @Field() email!: string +} export class InvalidEmail extends UserFriendlyError { - constructor(message?: string) { - super('invalid_input', 'invalid_email', message); + constructor(args: InvalidEmailDataType, message?: string | ((args: InvalidEmailDataType) => string)) { + super('invalid_input', 'invalid_email', message, args); } } @ObjectType() @@ -620,5 +624,5 @@ registerEnumType(ErrorNames, { export const ErrorDataUnionType = createUnionType({ name: 'ErrorDataUnion', types: () => - [UnknownOauthProviderDataType, MissingOauthQueryParameterDataType, InvalidPasswordLengthDataType, SpaceNotFoundDataType, NotInSpaceDataType, AlreadyInSpaceDataType, SpaceAccessDeniedDataType, SpaceOwnerNotFoundDataType, DocNotFoundDataType, DocAccessDeniedDataType, VersionRejectedDataType, InvalidHistoryTimestampDataType, DocHistoryNotFoundDataType, BlobNotFoundDataType, SubscriptionAlreadyExistsDataType, SubscriptionNotExistsDataType, SameSubscriptionRecurringDataType, SubscriptionPlanNotFoundDataType, CopilotMessageNotFoundDataType, CopilotPromptNotFoundDataType, CopilotProviderSideErrorDataType, RuntimeConfigNotFoundDataType, InvalidRuntimeConfigTypeDataType] as const, + [UnknownOauthProviderDataType, MissingOauthQueryParameterDataType, InvalidEmailDataType, InvalidPasswordLengthDataType, SpaceNotFoundDataType, NotInSpaceDataType, AlreadyInSpaceDataType, SpaceAccessDeniedDataType, SpaceOwnerNotFoundDataType, DocNotFoundDataType, DocAccessDeniedDataType, VersionRejectedDataType, InvalidHistoryTimestampDataType, DocHistoryNotFoundDataType, BlobNotFoundDataType, SubscriptionAlreadyExistsDataType, SubscriptionNotExistsDataType, SameSubscriptionRecurringDataType, SubscriptionPlanNotFoundDataType, CopilotMessageNotFoundDataType, CopilotPromptNotFoundDataType, CopilotProviderSideErrorDataType, RuntimeConfigNotFoundDataType, InvalidRuntimeConfigTypeDataType] as const, }); diff --git a/packages/backend/server/src/fundamentals/graphql/logger-plugin.ts b/packages/backend/server/src/fundamentals/graphql/logger-plugin.ts index 9bab7c0a20..66e758d34c 100644 --- a/packages/backend/server/src/fundamentals/graphql/logger-plugin.ts +++ b/packages/backend/server/src/fundamentals/graphql/logger-plugin.ts @@ -43,9 +43,12 @@ export class GQLLoggerPlugin implements ApolloServerPlugin { ); error.log('GraphQL'); - metrics.gql - .counter('query_error_counter') - .add(1, { operation, code: error.status }); + metrics.gql.counter('query_error_counter').add(1, { + operation, + code: error.status, + type: error.type, + error: error.name, + }); }); return Promise.resolve(); diff --git a/packages/backend/server/src/fundamentals/nestjs/exception.ts b/packages/backend/server/src/fundamentals/nestjs/exception.ts index e7a86e555e..3c524642a7 100644 --- a/packages/backend/server/src/fundamentals/nestjs/exception.ts +++ b/packages/backend/server/src/fundamentals/nestjs/exception.ts @@ -46,7 +46,9 @@ export class GlobalExceptionFilter extends BaseExceptionFilter { throw error; } else { error.log('HTTP'); - metrics.controllers.counter('error').add(1, { status: error.status }); + metrics.controllers + .counter('error') + .add(1, { status: error.status, type: error.type, error: error.name }); const res = host.switchToHttp().getResponse(); const respondText = res.getHeader('content-type') === 'text/plain'; diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index 5d129eb89e..c556082f1e 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -208,7 +208,7 @@ type EditorType { name: String! } -union ErrorDataUnion = AlreadyInSpaceDataType | BlobNotFoundDataType | CopilotMessageNotFoundDataType | CopilotPromptNotFoundDataType | CopilotProviderSideErrorDataType | DocAccessDeniedDataType | DocHistoryNotFoundDataType | DocNotFoundDataType | InvalidHistoryTimestampDataType | InvalidPasswordLengthDataType | InvalidRuntimeConfigTypeDataType | MissingOauthQueryParameterDataType | NotInSpaceDataType | RuntimeConfigNotFoundDataType | SameSubscriptionRecurringDataType | SpaceAccessDeniedDataType | SpaceNotFoundDataType | SpaceOwnerNotFoundDataType | SubscriptionAlreadyExistsDataType | SubscriptionNotExistsDataType | SubscriptionPlanNotFoundDataType | UnknownOauthProviderDataType | VersionRejectedDataType +union ErrorDataUnion = AlreadyInSpaceDataType | BlobNotFoundDataType | CopilotMessageNotFoundDataType | CopilotPromptNotFoundDataType | CopilotProviderSideErrorDataType | DocAccessDeniedDataType | DocHistoryNotFoundDataType | DocNotFoundDataType | InvalidEmailDataType | InvalidHistoryTimestampDataType | InvalidPasswordLengthDataType | InvalidRuntimeConfigTypeDataType | MissingOauthQueryParameterDataType | NotInSpaceDataType | RuntimeConfigNotFoundDataType | SameSubscriptionRecurringDataType | SpaceAccessDeniedDataType | SpaceNotFoundDataType | SpaceOwnerNotFoundDataType | SubscriptionAlreadyExistsDataType | SubscriptionNotExistsDataType | SubscriptionPlanNotFoundDataType | UnknownOauthProviderDataType | VersionRejectedDataType enum ErrorNames { ACCESS_DENIED @@ -315,6 +315,10 @@ type HumanReadableQuotaType { storageQuota: String! } +type InvalidEmailDataType { + email: String! +} + type InvalidHistoryTimestampDataType { timestamp: String! } diff --git a/packages/backend/server/tests/auth/controller.spec.ts b/packages/backend/server/tests/auth/controller.spec.ts index bedf7f783e..7204fa2f7e 100644 --- a/packages/backend/server/tests/auth/controller.spec.ts +++ b/packages/backend/server/tests/auth/controller.spec.ts @@ -123,7 +123,7 @@ test('should not be able to sign in if email is invalid', async t => { .send({ email: '' }) .expect(400); - t.is(res.body.message, 'An invalid email provided.'); + t.is(res.body.message, 'An invalid email provided: '); }); test('should not be able to sign in if forbidden', async t => {