From ab69e113f4921462b72f1f352dfefe52b37862f5 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Thu, 6 Jun 2024 11:20:54 +0900 Subject: enhance(backend): improve sentry integration --- packages/backend/src/server/api/ApiCallService.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 271ef80554..e21a5e90ab 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -93,7 +93,7 @@ export class ApiCallService implements OnApplicationShutdown { } } - #onExecError(ep: IEndpoint, data: any, err: Error): void { + #onExecError(ep: IEndpoint, data: any, err: Error, userId?: MiUser['id']): void { if (err instanceof ApiError || err instanceof AuthenticationError) { throw err; } else { @@ -108,10 +108,12 @@ export class ApiCallService implements OnApplicationShutdown { id: errId, }, }); - console.error(err, errId); if (this.config.sentryForBackend) { Sentry.captureMessage(`Internal error occurred in ${ep.name}: ${err.message}`, { + user: { + id: userId, + }, extra: { ep: ep.name, ps: data, @@ -410,9 +412,13 @@ export class ApiCallService implements OnApplicationShutdown { // API invoking if (this.config.sentryForBackend) { - return await Sentry.startSpan({ name: 'API: ' + ep.name }, () => ep.exec(data, user, token, file, request.ip, request.headers).catch((err: Error) => this.#onExecError(ep, data, err))); + return await Sentry.startSpan({ + name: 'API: ' + ep.name, + }, () => ep.exec(data, user, token, file, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id))); } else { - return await ep.exec(data, user, token, file, request.ip, request.headers).catch((err: Error) => this.#onExecError(ep, data, err)); + return await ep.exec(data, user, token, file, request.ip, request.headers) + .catch((err: Error) => this.#onExecError(ep, data, err, user?.id)); } } -- cgit v1.2.3-freya From 859271613958cc4a9bc8fcd9616c3146c07a50a4 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Fri, 7 Jun 2024 13:15:37 +0900 Subject: enhance(backend): improve sentry integration --- packages/backend/src/queue/QueueProcessorService.ts | 7 +++++++ packages/backend/src/server/api/ApiCallService.ts | 1 + 2 files changed, 8 insertions(+) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/queue/QueueProcessorService.ts b/packages/backend/src/queue/QueueProcessorService.ts index 6a87be021e..7bfe1f4caa 100644 --- a/packages/backend/src/queue/QueueProcessorService.ts +++ b/packages/backend/src/queue/QueueProcessorService.ts @@ -169,6 +169,7 @@ export class QueueProcessorService implements OnApplicationShutdown { systemLogger.error(`failed(${err.stack}) id=${job ? job.id : '-'}`, { job, e: renderError(err) }); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: System: ${job?.name ?? '?'}: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -225,6 +226,7 @@ export class QueueProcessorService implements OnApplicationShutdown { dbLogger.error(`failed(${err.stack}) id=${job ? job.id : '-'}`, { job, e: renderError(err) }); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: DB: ${job?.name ?? '?'}: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -264,6 +266,7 @@ export class QueueProcessorService implements OnApplicationShutdown { deliverLogger.error(`failed(${err.stack}) ${getJobInfo(job)} to=${job ? job.data.to : '-'}`); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: Deliver: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -303,6 +306,7 @@ export class QueueProcessorService implements OnApplicationShutdown { inboxLogger.error(`failed(${err.stack}) ${getJobInfo(job)} activity=${job ? (job.data.activity ? job.data.activity.id : 'none') : '-'}`, { job, e: renderError(err) }); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: Inbox: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -342,6 +346,7 @@ export class QueueProcessorService implements OnApplicationShutdown { webhookLogger.error(`failed(${err.stack}) ${getJobInfo(job)} to=${job ? job.data.to : '-'}`); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: WebhookDeliver: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -388,6 +393,7 @@ export class QueueProcessorService implements OnApplicationShutdown { relationshipLogger.error(`failed(${err.stack}) id=${job ? job.id : '-'}`, { job, e: renderError(err) }); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: Relationship: ${job?.name ?? '?'}: ${err.message}`, { + level: 'error', extra: { job, err }, }); } @@ -428,6 +434,7 @@ export class QueueProcessorService implements OnApplicationShutdown { objectStorageLogger.error(`failed(${err.stack}) id=${job ? job.id : '-'}`, { job, e: renderError(err) }); if (config.sentryForBackend) { Sentry.captureMessage(`Queue: ObjectStorage: ${job?.name ?? '?'}: ${err.message}`, { + level: 'error', extra: { job, err }, }); } diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index e21a5e90ab..166f9c8675 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -111,6 +111,7 @@ export class ApiCallService implements OnApplicationShutdown { if (this.config.sentryForBackend) { Sentry.captureMessage(`Internal error occurred in ${ep.name}: ${err.message}`, { + level: 'error', user: { id: userId, }, -- cgit v1.2.3-freya From dc3629e732e5aefd792452f0b43a7bb7fdaf103e Mon Sep 17 00:00:00 2001 From: Kisaragi <48310258+KisaragiEffective@users.noreply.github.com> Date: Thu, 13 Jun 2024 10:56:26 +0900 Subject: feat(backend): report `Retry-After` if client hit rate limit (#13949) * feat(backend): report `Retry-After` if client hit rate limit * refactor(backend): fix lint error --- packages/backend/src/server/api/ApiCallService.ts | 27 ++++++++++++---- .../backend/src/server/api/RateLimiterService.ts | 36 ++++++++++++---------- 2 files changed, 40 insertions(+), 23 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 166f9c8675..47f64f6609 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -73,6 +73,16 @@ export class ApiCallService implements OnApplicationShutdown { reply.header('WWW-Authenticate', `Bearer realm="Misskey", error="insufficient_scope", error_description="${err.message}"`); } statusCode = statusCode ?? 403; + } else if (err.code === 'RATE_LIMIT_EXCEEDED') { + const info: unknown = err.info; + const unixEpochInSeconds = Date.now(); + if (typeof(info) === 'object' && info && 'resetMs' in info && typeof(info.resetMs) === 'number') { + const cooldownInSeconds = Math.ceil((info.resetMs - unixEpochInSeconds) / 1000); + // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく + reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); + } else { + this.logger.warn(`rate limit information has unexpected type ${typeof(err.info?.reset)}`); + } } else if (!statusCode) { statusCode = 500; } @@ -308,12 +318,17 @@ export class ApiCallService implements OnApplicationShutdown { if (factor > 0) { // Rate limit await this.rateLimiterService.limit(limit as IEndpointMeta['limit'] & { key: NonNullable }, limitActor, factor).catch(err => { - throw new ApiError({ - message: 'Rate limit exceeded. Please try again later.', - code: 'RATE_LIMIT_EXCEEDED', - id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', - httpStatusCode: 429, - }); + if ('info' in err) { + // errはLimiter.LimiterInfoであることが期待される + throw new ApiError({ + message: 'Rate limit exceeded. Please try again later.', + code: 'RATE_LIMIT_EXCEEDED', + id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', + httpStatusCode: 429, + }, err.info); + } else { + throw new TypeError('information must be a rate-limiter information.'); + } }); } } diff --git a/packages/backend/src/server/api/RateLimiterService.ts b/packages/backend/src/server/api/RateLimiterService.ts index 0439cdfe5e..cae106c273 100644 --- a/packages/backend/src/server/api/RateLimiterService.ts +++ b/packages/backend/src/server/api/RateLimiterService.ts @@ -32,11 +32,13 @@ export class RateLimiterService { @bindThis public limit(limitation: IEndpointMeta['limit'] & { key: NonNullable }, actor: string, factor = 1) { - return new Promise((ok, reject) => { - if (this.disabled) ok(); + { + if (this.disabled) { + return Promise.resolve(); + } // Short-term limit - const min = (): void => { + const min = new Promise((ok, reject) => { const minIntervalLimiter = new Limiter({ id: `${actor}:${limitation.key}:min`, duration: limitation.minInterval! * factor, @@ -46,25 +48,25 @@ export class RateLimiterService { minIntervalLimiter.get((err, info) => { if (err) { - return reject('ERR'); + return reject({ code: 'ERR', info }); } this.logger.debug(`${actor} ${limitation.key} min remaining: ${info.remaining}`); if (info.remaining === 0) { - reject('BRIEF_REQUEST_INTERVAL'); + return reject({ code: 'BRIEF_REQUEST_INTERVAL', info }); } else { if (hasLongTermLimit) { - max(); + return max; } else { - ok(); + return ok(); } } }); - }; + }); // Long term limit - const max = (): void => { + const max = new Promise((ok, reject) => { const limiter = new Limiter({ id: `${actor}:${limitation.key}`, duration: limitation.duration! * factor, @@ -74,18 +76,18 @@ export class RateLimiterService { limiter.get((err, info) => { if (err) { - return reject('ERR'); + return reject({ code: 'ERR', info }); } this.logger.debug(`${actor} ${limitation.key} max remaining: ${info.remaining}`); if (info.remaining === 0) { - reject('RATE_LIMIT_EXCEEDED'); + return reject({ code: 'RATE_LIMIT_EXCEEDED', info }); } else { - ok(); + return ok(); } }); - }; + }); const hasShortTermLimit = typeof limitation.minInterval === 'number'; @@ -94,12 +96,12 @@ export class RateLimiterService { typeof limitation.max === 'number'; if (hasShortTermLimit) { - min(); + return min; } else if (hasLongTermLimit) { - max(); + return max; } else { - ok(); + return Promise.resolve(); } - }); + } } } -- cgit v1.2.3-freya