fix: assign correct sentry scoping (#272)

Updates sentry to properly create scopes for handling each command, as well as the event based handlers like reactions. Bumps the sentry group with 2 updates in the / directory: [@sentry/node](https://github.com/getsentry/sentry-javascript) and [@sentry/profiling-node](https://github.com/getsentry/sentry-javascript). Updates `@sentry/node` from 7.113.0 to 8.1.0 - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/develop/CHANGELOG.md) - [Commits](getsentry/sentry-javascript@7.113.0...8.1.0) Updates `@sentry/profiling-node` from 7.113.0 to 8.1.0 - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/develop/CHANGELOG.md) - [Commits](getsentry/sentry-javascript@7.113.0...8.1.0) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-major dependency-group: sentry - dependency-name: "@sentry/profiling-node" dependency-type: direct:production update-type: version-update:semver-major dependency-group: sentry ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ssilve1989 · May 16, 2024 · f38d1e1 · f38d1e1
1 parent 9eb9ac7
commit f38d1e1
Show file tree

Hide file tree

Showing 11 changed files with 869 additions and 179 deletions.
diff --git a/biome.json b/biome.json
@@ -15,7 +15,8 @@
       "style": {
         "noNonNullAssertion": "off",
         "useImportType": "off",
-        "noParameterProperties": "off"
+        "noParameterProperties": "off",
+        "noNamespaceImport": "off"
       },
       "suspicious": {
         "noExplicitAny": "off"

diff --git a/package.json b/package.json
@@ -33,8 +33,8 @@
     "@nestjs/core": "^10.3.8",
     "@nestjs/cqrs": "^10.2.7",
     "@sentry/cli": "^2.31.2",
-    "@sentry/node": "^7.113.0",
-    "@sentry/profiling-node": "^7.113.0",
+    "@sentry/node": "^8.2.1",
+    "@sentry/profiling-node": "^8.2.1",
     "class-transformer": "^0.5.1",
     "class-validator": "^0.14.1",
     "dayjs": "^1.11.11",

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/commands/settings/subcommands/edit-settings-command.handler.ts b/src/commands/settings/subcommands/edit-settings-command.handler.ts
@@ -40,13 +40,7 @@ class EditSettingsCommandHandler
   }
 
   private handleError(e: unknown, interaction: ChatInputCommandInteraction) {
-    sentryReport(e, {
-      userId: interaction.user.id,
-      extra: {
-        command: interaction.command?.name,
-      },
-    });
-
+    sentryReport(e);
     this.logger.error(e);
     return interaction.editReply('Something went wrong!');
   }

diff --git a/src/commands/signup/signup-command.handler.ts b/src/commands/signup/signup-command.handler.ts
@@ -217,10 +217,7 @@ class SignupCommandHandler implements ICommandHandler<SignupCommand> {
     error: unknown,
     interaction: ChatInputCommandInteraction,
   ) {
-    sentryReport(error, {
-      userId: interaction.user.id,
-      extra: { command: interaction.command?.name },
-    });
+    sentryReport(error);
 
     this.logger.error(error);
 

diff --git a/src/commands/signup/signup.service.ts b/src/commands/signup/signup.service.ts
@@ -4,6 +4,7 @@ import {
   OnApplicationBootstrap,
   OnModuleDestroy,
 } from '@nestjs/common';
+import * as Sentry from '@sentry/node';
 import {
   ActionRowBuilder,
   Colors,
@@ -78,33 +79,47 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
         mergeMap((group$) =>
           group$.pipe(
             concatMap(async (event) => {
-              if (!event.reaction.message.inGuild()) return EMPTY;
-
-              try {
-                // TODO: dangerous cast to Settings, but know its safe from current usage
-                // attempts to type it correctly just result in weirdness since all the other fields on the object are optional
-                const [reaction, user, settings = {} as SettingsDocument] =
-                  await Promise.all([
-                    hydrateReaction(event.reaction),
-                    hydrateUser(event.user),
-                    this.settingsCollection.getSettings(
-                      event.reaction.message.guildId,
-                    ),
-                  ]);
-
-                // TODO: We can extract the type of the Message to be `Message<True>` since shouldHandleReaction checks if the message is inGuild()
-                const shouldHandle = await this.shouldHandleReaction(
-                  reaction,
-                  user,
-                  settings,
-                );
-
-                return shouldHandle
-                  ? await this.handleReaction(reaction, user, settings)
-                  : EMPTY;
-              } catch (error) {
-                this.handleError(error, event.user, event.reaction.message);
-              }
+              // TODO: cleanup this anon function
+              await Sentry.withScope(async (scope) => {
+                if (!event.reaction.message.inGuild()) {
+                  return EMPTY;
+                }
+
+                scope.setUser({
+                  id: event.user.id,
+                  username: event.user.username ?? 'unknown',
+                });
+
+                scope.setExtras({
+                  message: getMessageLink(event.reaction.message),
+                });
+
+                try {
+                  // TODO: dangerous cast to Settings, but know its safe from current usage
+                  // attempts to type it correctly just result in weirdness since all the other fields on the object are optional
+                  const [reaction, user, settings = {} as SettingsDocument] =
+                    await Promise.all([
+                      hydrateReaction(event.reaction),
+                      hydrateUser(event.user),
+                      this.settingsCollection.getSettings(
+                        event.reaction.message.guildId,
+                      ),
+                    ]);
+
+                  // TODO: We can extract the type of the Message to be `Message<True>` since shouldHandleReaction checks if the message is inGuild()
+                  const shouldHandle = await this.shouldHandleReaction(
+                    reaction,
+                    user,
+                    settings,
+                  );
+
+                  return shouldHandle
+                    ? await this.handleReaction(reaction, user, settings)
+                    : EMPTY;
+                } catch (error) {
+                  this.handleError(error, event.user, event.reaction.message);
+                }
+              });
             }),
           ),
         ),
@@ -285,20 +300,15 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
     user: User | PartialUser,
     message: Message | PartialMessage,
   ) {
-    sentryReport(error, {
-      userId: user.username || 'unknown',
-      extra: { message: getMessageLink(message) },
-    });
+    const scope = Sentry.getCurrentScope();
+    scope.setUser({ id: user.username ?? 'unknown' });
+    scope.setExtras({ message: getMessageLink(message) });
 
     this.logger.error(error);
 
-    const reply = match(error)
-      .with(
-        P.instanceOf(DiscordjsError),
-        ({ code }) => code === DiscordjsErrorCodes.InteractionCollectorError,
-        () => SIGNUP_MESSAGES.PROG_SELECTION_TIMEOUT,
-      )
-      .otherwise(() => SIGNUP_MESSAGES.GENERIC_APPROVAL_ERROR);
+    const reply = SIGNUP_MESSAGES.GENERIC_APPROVAL_ERROR;
+
+    scope.captureMessage(reply, 'info');
 
     // TODO: Improve error reporting to better inform user what happened
     await Promise.all([
@@ -335,7 +345,8 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
 
     try {
       const reply = await message.awaitMessageComponent({
-        time: 60_000,
+        // time: 60_000 * 2, // 2 minutes
+        time: 5_000,
         filter: isSameUserFilter(user),
       });
 
@@ -351,10 +362,11 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
 
       await reply.update(SIGNUP_MESSAGES.UNEXPECTED_PROG_SELECTION_ERROR);
     } catch (error) {
-      sentryReport(error, {
-        userId: user.username,
-        extra: { encounter: signup.encounter, discordId: signup.discordId },
-      });
+      sentryReport(error, (scope) =>
+        scope.setExtras({
+          extra: { encounter: signup.encounter, discordId: signup.discordId },
+        }),
+      );
 
       this.logger.error(error);
 
@@ -393,7 +405,9 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
       partyType === PartyType.EARLY_PROG_PARTY ||
       partyType === PartyType.PROG_PARTY;
 
-    if (!(role && isValidPartyType)) return;
+    if (!(role && isValidPartyType)) {
+      return;
+    }
 
     try {
       const member = await this.discordService.getGuildMember(
@@ -405,7 +419,7 @@ class SignupService implements OnApplicationBootstrap, OnModuleDestroy {
         await member.roles.add(role);
       }
     } catch (e) {
-      sentryReport(e, { extra: { encounter, discordId } });
+      sentryReport(e, (scope) => scope.setExtras({ encounter, discordId }));
       this.logger.error(e);
     }
   }

diff --git a/src/commands/slash-commands.service.ts b/src/commands/slash-commands.service.ts
@@ -1,6 +1,7 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { CommandBus } from '@nestjs/cqrs';
+import * as Sentry from '@sentry/node';
 import {
   ChatInputCommandInteraction,
   Client,
@@ -38,33 +39,43 @@ class SlashCommandsService {
 
   listenToCommands() {
     this.client.on(Events.InteractionCreate, async (interaction) => {
-      if (!(interaction.isChatInputCommand() && interaction.inGuild())) return;
+      await Sentry.withScope(async (scope) => {
+        if (!(interaction.isChatInputCommand() && interaction.inGuild())) {
+          return;
+        }
+
+        scope.setUser({ userId: interaction.user.id });
+        scope.setExtras({
+          username: interaction.user.username,
+          command: interaction.commandName,
+        });
 
-      // TODO: This could be more generic somehow
-      const command = match(interaction.commandName)
-        .with(LookupSlashCommand.name, () => new LookupCommand(interaction))
-        .with(SignupSlashCommand.name, () => new SignupCommand(interaction))
-        .with(StatusSlashCommand.name, () => new StatusCommand(interaction))
-        .with(SettingsSlashCommand.name, () => {
-          const subcommand = interaction.options.getSubcommand();
-          return match(subcommand)
-            .with('edit', () => new EditSettingsCommand(interaction))
-            .with('view', () => new ViewSettingsCommand(interaction))
-            .run();
-        })
-        .with(
-          RemoveSignupSlashCommand.name,
-          () => new RemoveSignupCommand(interaction),
-        )
-        .otherwise(() => undefined);
+        // TODO: This could be more generic somehow
+        const command = match(interaction.commandName)
+          .with(LookupSlashCommand.name, () => new LookupCommand(interaction))
+          .with(SignupSlashCommand.name, () => new SignupCommand(interaction))
+          .with(StatusSlashCommand.name, () => new StatusCommand(interaction))
+          .with(SettingsSlashCommand.name, () => {
+            const subcommand = interaction.options.getSubcommand();
+            return match(subcommand)
+              .with('edit', () => new EditSettingsCommand(interaction))
+              .with('view', () => new ViewSettingsCommand(interaction))
+              .run();
+          })
+          .with(
+            RemoveSignupSlashCommand.name,
+            () => new RemoveSignupCommand(interaction),
+          )
+          .otherwise(() => undefined);
 
-      if (command) {
-        try {
-          await this.commandBus.execute(command);
-        } catch (err) {
-          await this.handleCommandError(err, interaction);
+        if (command) {
+          try {
+            await this.commandBus.execute(command);
+          } catch (err) {
+            await this.handleCommandError(err, interaction);
+          }
         }
-      }
+      });
     });
   }
 

diff --git a/src/commands/status/status-command.handler.ts b/src/commands/status/status-command.handler.ts
@@ -67,10 +67,7 @@ class StatusCommandHandler implements ICommandHandler<StatusCommand> {
     error: unknown,
     interaction: ChatInputCommandInteraction,
   ) {
-    sentryReport(error, {
-      userId: interaction.user.username,
-      extra: { command: interaction.command?.name },
-    });
+    sentryReport(error);
 
     this.logger.error(error);
 

diff --git a/src/discord/discord.module.ts b/src/discord/discord.module.ts
@@ -5,10 +5,10 @@ import {
   OnApplicationShutdown,
 } from '@nestjs/common';
 import { ConfigModule, ConfigService } from '@nestjs/config';
+import * as Sentry from '@sentry/node';
 import { ActivityType, Client, Events, Options } from 'discord.js';
 import { first, firstValueFrom, fromEvent } from 'rxjs';
 import { AppConfig } from '../app.config.js';
-import { sentryReport } from '../sentry/sentry.consts.js';
 import { INTENTS, PARTIALS } from './discord.consts.js';
 import { DISCORD_CLIENT, InjectDiscordClient } from './discord.decorators.js';
 import { CacheTime } from './discord.helpers.js';
@@ -59,7 +59,7 @@ import { DiscordService } from './discord.service.js';
         const started$ = fromEvent(client, Events.ClientReady).pipe(first());
 
         client.once('error', (error) => {
-          sentryReport(error);
+          Sentry.captureException(error);
           logger.error(error);
         });
 
@@ -86,7 +86,7 @@ class DiscordModule implements OnApplicationBootstrap, OnApplicationShutdown {
     fromEvent(this.client, Events.CacheSweep).subscribe({
       next: (msg) => this.logger.log(msg),
       error: (err) => {
-        sentryReport(err);
+        Sentry;
         this.logger.error(err);
       },
     });

diff --git a/src/sentry/sentry.consts.ts b/src/sentry/sentry.consts.ts
@@ -1,16 +1,13 @@
 import * as Sentry from '@sentry/node';
 
-interface ReportOptions {
-  userId?: string;
-  extra?: Record<string, any>;
-}
+type ScopeFn = (scope: Sentry.Scope) => void;
 
-export const sentryReport = (
-  error: unknown,
-  { userId, extra }: ReportOptions = {},
-) => {
-  Sentry.withScope((scope) => {
-    userId && scope.setUser({ id: userId });
-    Sentry.captureException(error, { extra });
-  });
+/**
+ * @param error The error to report
+ * @param scopeFn Optional function to modify the scope before sending the report
+ */
+export const sentryReport = (error: unknown, scopeFn?: ScopeFn) => {
+  const scope = Sentry.getCurrentScope();
+  scope.captureException(error);
+  scopeFn?.(scope);
 };
diff --git a/src/sheets/sheets.service.ts b/src/sheets/sheets.service.ts
@@ -78,8 +78,7 @@ class SheetsService {
         );
 
       default: {
-        const msg = `unknown party type: ${partyType} for user: ${signup.discordId}. Not appending to any Google Sheet`;
-        Sentry.captureMessage(msg);
+        const msg = `unknown party type: ${partyType} for character: ${signup.character}`;
         this.logger.warn(msg);
       }
     }