Skip to content

Commit

Permalink
refactor: assign correct sentry scoping
Browse files Browse the repository at this point in the history
---
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]>
  • Loading branch information
dependabot[bot] authored and ssilve1989 committed May 16, 2024
1 parent 9eb9ac7 commit 6797ed6
Show file tree
Hide file tree
Showing 11 changed files with 869 additions and 179 deletions.
3 changes: 2 additions & 1 deletion biome.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"style": {
"noNonNullAssertion": "off",
"useImportType": "off",
"noParameterProperties": "off"
"noParameterProperties": "off",
"noNamespaceImport": "off"
},
"suspicious": {
"noExplicitAny": "off"
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
830 changes: 755 additions & 75 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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!');
}
Expand Down
5 changes: 1 addition & 4 deletions src/commands/signup/signup-command.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
104 changes: 59 additions & 45 deletions src/commands/signup/signup.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
OnApplicationBootstrap,
OnModuleDestroy,
} from '@nestjs/common';
import * as Sentry from '@sentry/node';
import {
ActionRowBuilder,
Colors,
Expand Down Expand Up @@ -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);
}
});
}),
),
),
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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),
});

Expand All @@ -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);

Expand Down Expand Up @@ -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(
Expand All @@ -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);
}
}
Expand Down
59 changes: 35 additions & 24 deletions src/commands/slash-commands.service.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
}
}
}
});
});
}

Expand Down
5 changes: 1 addition & 4 deletions src/commands/status/status-command.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions src/discord/discord.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});

Expand All @@ -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);
},
});
Expand Down
21 changes: 9 additions & 12 deletions src/sentry/sentry.consts.ts
Original file line number Diff line number Diff line change
@@ -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);
};
3 changes: 1 addition & 2 deletions src/sheets/sheets.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 6797ed6

Please sign in to comment.