Conversation
WalkthroughThis update introduces comprehensive support for Discord thread events by adding new handler logic, persistence functions, and mapping improvements for thread entities. It also refactors database error handling for gateway activities, updates a dependency, modifies import formatting, and adjusts event handling to log unsupported events instead of throwing errors. Changes
Sequence Diagram(s)sequenceDiagram
participant DiscordGateway as Discord Gateway
participant Workflow as Gateway Workflow
participant Handler as ThreadHandler
participant Activity as Persistence Activity
DiscordGateway->>Workflow: ThreadCreate/Update/Delete Event
Workflow->>Handler: Call create/update/delete(data)
Handler->>Activity: createThread/updateThread/deleteThread(guildId, data)
Activity-->>Handler: (Persistence completed)
Handler-->>Workflow: (Handler completed)
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/activities/discord/gateway/mapping.activity.ts (1)
2-10: Long import list hurts readabilityThe combined import now exceeds 120 chars and is hard to diff.
Consider grouping related types or usingimport type { … }with line-breaks to respect the project’s prettier / eslint max-length rule.Also applies to: 17-17
src/workflows/discord/gateway/index.ts (1)
12-13: Log should include event name for observabilityThe warning currently lacks context. Add the unsupported event type so on-call can triage quickly.
- log.warn('Unsupported gateway event'); + log.warn(`Unsupported gateway event: ${event as string}`);src/workflows/discord/gateway/handlers/thread.handler.ts (1)
15-27: Consider refactoring to standalone functions instead of a static-only class.Static-only classes are an anti-pattern in JavaScript/TypeScript. Consider refactoring to standalone functions for better tree-shaking and cleaner code.
-export class ThreadHandler { - static async create(data: GatewayThreadCreateDispatchData): Promise<void> { - await activitiesProxy.createThread(data.guild_id!, data); - } - - static async update(data: GatewayThreadUpdateDispatchData): Promise<void> { - await activitiesProxy.updateThread(data.guild_id!, { id: data.id }, data); - } - - static async delete(data: GatewayThreadDeleteDispatchData): Promise<void> { - await activitiesProxy.deleteThread(data.guild_id!, data.id); - } -} +export async function createThread(data: GatewayThreadCreateDispatchData): Promise<void> { + await activitiesProxy.createThread(data.guild_id!, data); +} + +export async function updateThread(data: GatewayThreadUpdateDispatchData): Promise<void> { + await activitiesProxy.updateThread(data.guild_id!, { id: data.id }, data); +} + +export async function deleteThread(data: GatewayThreadDeleteDispatchData): Promise<void> { + await activitiesProxy.deleteThread(data.guild_id!, data.id); +}Then update the imports in
src/workflows/discord/gateway/handlers/index.ts:-import { ThreadHandler } from './thread.handler'; +import * as ThreadHandler from './thread.handler';🧰 Tools
🪛 Biome (1.9.4)
[error] 15-27: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/activities/discord/gateway/persistence.activity.ts (1)
15-30: Replacevoidwithundefinedin the return type.Using
voidin a union type can be confusing. Useundefinedfor better clarity.async function handleDuplicateKeyError<T>( operation: () => Promise<T>, entityName: string, entityId: string, guildId: Snowflake, -): Promise<T | void> { +): Promise<T | undefined> {🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.gitignore(1 hunks)package.json(1 hunks)src/activities/discord/gateway/mapping.activity.ts(1 hunks)src/activities/discord/gateway/persistence.activity.ts(3 hunks)src/workflows/discord/gateway/handlers/index.ts(2 hunks)src/workflows/discord/gateway/handlers/message.handler.ts(2 hunks)src/workflows/discord/gateway/handlers/thread.handler.ts(1 hunks)src/workflows/discord/gateway/index.ts(1 hunks)src/workflows/discord/gateway/mappers/message.mapper.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/workflows/discord/gateway/index.ts (1)
src/workflows/discord/gateway/handlers/index.ts (1)
eventHandlers(9-39)
src/workflows/discord/gateway/handlers/index.ts (1)
src/workflows/discord/gateway/handlers/thread.handler.ts (1)
ThreadHandler(15-27)
src/activities/discord/gateway/persistence.activity.ts (1)
src/config/logger.config.ts (1)
logger(4-10)
🪛 Biome (1.9.4)
src/workflows/discord/gateway/handlers/thread.handler.ts
[error] 15-27: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/activities/discord/gateway/persistence.activity.ts
[error] 20-20: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-push
🔇 Additional comments (8)
.gitignore (1)
6-8: Ignore rule additions look goodAdding
.cursor/*and.cursoris harmless and shields dev artefacts from VCS. No concerns.package.json (1)
40-40: Verify@togethercrew.dev/dbupgrade before mergeJumping three minor versions (
3.10.x➜3.13.x) may introduce breaking schema or API tweaks.
Please:
- run the unit / integration suite against the new version;
- regenerate
pnpm-lock.yaml/ lock-file and commit it;- skim the changelog for migration notes.
src/workflows/discord/gateway/handlers/message.handler.ts (1)
2-5: Import re-formatting LGTMPure style change – no behavioural impact.
src/workflows/discord/gateway/index.ts (1)
3-4: 👍 Using Temporallogis appropriateSwitching from thrown errors to workflow-safe logging is sensible.
src/workflows/discord/gateway/handlers/index.ts (1)
7-7: LGTM! Thread handlers properly integrated.The thread event handlers follow the established pattern and are correctly mapped to their respective handler methods.
Also applies to: 33-38
src/workflows/discord/gateway/mappers/message.mapper.ts (1)
47-50: Thread detection logic is correctly implemented.The implementation properly identifies Discord thread channel types (10: news thread, 11: public thread, 12: private thread) and correctly assigns the channel_id to either channelId or threadId fields.
Also applies to: 64-67
src/activities/discord/gateway/persistence.activity.ts (2)
15-59: Excellent refactoring to reduce code duplication.The introduction of
handleDuplicateKeyErrorandperformUpserthelper functions effectively centralizes error handling and upsert logic, making the codebase more maintainable and consistent.🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
263-281: Thread deletion implementation looks good.The use of
deleteManyis appropriate for thread deletion, and the error handling with proper logging follows the established pattern.
| authorId?: string, | ||
| ): Promise<boolean> { | ||
| if (!(await activities.isChannelSelected(guildId, channelId))) return false; | ||
| // if (!(await activities.isChannelSelected(guildId, channelId))) return false; |
There was a problem hiding this comment.
Channel-selection guard silently disabled
Commenting out the isChannelSelected check means all messages, including those from un-whitelisted channels, will now be processed. This may:
- flood the persistence layer,
- undermine privacy filters,
- inflate cost of downstream analyses.
Unless this is a deliberate feature toggle, please reinstate the guard or gate it behind a config flag.
- // if (!(await activities.isChannelSelected(guildId, channelId))) return false;
+ if (!(await activities.isChannelSelected(guildId, channelId))) return false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // if (!(await activities.isChannelSelected(guildId, channelId))) return false; | |
| if (!(await activities.isChannelSelected(guildId, channelId))) return false; |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/handlers/message.handler.ts at line 21, the
channel-selection guard check using isChannelSelected is commented out, causing
all messages to be processed regardless of channel whitelist. To fix this,
reinstate the isChannelSelected check to ensure only messages from selected
channels are processed, or alternatively, wrap this check in a configuration
flag to enable or disable it explicitly. This will prevent processing unwanted
messages and avoid flooding the persistence layer or violating privacy
constraints.
|
|
||
| export class ThreadHandler { | ||
| static async create(data: GatewayThreadCreateDispatchData): Promise<void> { | ||
| await activitiesProxy.createThread(data.guild_id!, data); |
There was a problem hiding this comment.
Validate guild_id instead of using non-null assertions.
The non-null assertions on guild_id could cause runtime errors. Consider adding validation to ensure safer code execution.
export class ThreadHandler {
static async create(data: GatewayThreadCreateDispatchData): Promise<void> {
+ if (!data.guild_id) {
+ throw new Error('Thread create event missing guild_id');
+ }
- await activitiesProxy.createThread(data.guild_id!, data);
+ await activitiesProxy.createThread(data.guild_id, data);
}
static async update(data: GatewayThreadUpdateDispatchData): Promise<void> {
+ if (!data.guild_id) {
+ throw new Error('Thread update event missing guild_id');
+ }
- await activitiesProxy.updateThread(data.guild_id!, { id: data.id }, data);
+ await activitiesProxy.updateThread(data.guild_id, { id: data.id }, data);
}
static async delete(data: GatewayThreadDeleteDispatchData): Promise<void> {
+ if (!data.guild_id) {
+ throw new Error('Thread delete event missing guild_id');
+ }
- await activitiesProxy.deleteThread(data.guild_id!, data.id);
+ await activitiesProxy.deleteThread(data.guild_id, data.id);
}
}Also applies to: 21-21, 25-25
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/handlers/thread.handler.ts at lines 17, 21, and
25, the code uses non-null assertions on guild_id which can cause runtime errors
if guild_id is undefined or null. Replace these assertions by adding explicit
validation checks to confirm guild_id is defined before calling createThread or
other functions using it. Handle the case where guild_id is missing gracefully,
such as by logging an error or returning early, to ensure safer code execution.
| console.log(isThreadMessage, payload.channel_type); | ||
| console.log('mama'); |
There was a problem hiding this comment.
Remove debug console.log statements.
Debug statements should not be committed to production code.
- console.log(isThreadMessage, payload.channel_type);
- console.log('mama');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log(isThreadMessage, payload.channel_type); | |
| console.log('mama'); |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/mappers/message.mapper.ts at lines 52 to 53,
remove the debug console.log statements that output isThreadMessage,
payload.channel_type, and the string 'mama' to clean up the production code and
avoid unnecessary logging.
| export function mapMessageCreate( | ||
| payload: GatewayMessageCreateDispatchData, | ||
| // payload: GatewayMessageCreateDispatchData, | ||
| payload: any, | ||
| ): IRawInfo { |
There was a problem hiding this comment.
Restore type safety by using the proper Discord type.
Changing the parameter type to any removes valuable type safety. This could lead to runtime errors and makes the code harder to maintain.
export function mapMessageCreate(
- // payload: GatewayMessageCreateDispatchData,
- payload: any,
+ payload: GatewayMessageCreateDispatchData,
): IRawInfo {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function mapMessageCreate( | |
| payload: GatewayMessageCreateDispatchData, | |
| // payload: GatewayMessageCreateDispatchData, | |
| payload: any, | |
| ): IRawInfo { | |
| export function mapMessageCreate( | |
| payload: GatewayMessageCreateDispatchData, | |
| ): IRawInfo { |
🤖 Prompt for AI Agents
In src/workflows/discord/gateway/mappers/message.mapper.ts around lines 43 to
46, the function parameter type was changed to 'any', removing type safety.
Restore the original Discord-specific type (likely
GatewayMessageCreateDispatchData) for the payload parameter to ensure proper
type checking and maintainability. Import the correct type if necessary and
replace 'any' with it.
| export async function createThread( | ||
| guildId: Snowflake, | ||
| data: IThread, | ||
| ): Promise<void> { | ||
| try { | ||
| const dbConnection = | ||
| await DatabaseManager.getInstance().getGuildDb(guildId); | ||
| const repository = makeThreadRepository(dbConnection); | ||
| await repository.create(data); | ||
| logger.debug({ guildId, threadId: data.id }, 'Created thread'); | ||
| } catch (err: any) { | ||
| if (err.code === 11000) { | ||
| logger.warn({ guildId, threadId: data.id }, 'Thread already exists'); | ||
| } else { | ||
| logger.error( | ||
| { err, guildId, threadId: data.id }, | ||
| 'Failed to create thread', | ||
| ); | ||
| throw err; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor createThread to use the handleDuplicateKeyError helper.
For consistency with other create functions, use the handleDuplicateKeyError helper function.
export async function createThread(
guildId: Snowflake,
data: IThread,
): Promise<void> {
- try {
- const dbConnection =
- await DatabaseManager.getInstance().getGuildDb(guildId);
- const repository = makeThreadRepository(dbConnection);
- await repository.create(data);
- logger.debug({ guildId, threadId: data.id }, 'Created thread');
- } catch (err: any) {
- if (err.code === 11000) {
- logger.warn({ guildId, threadId: data.id }, 'Thread already exists');
- } else {
- logger.error(
- { err, guildId, threadId: data.id },
- 'Failed to create thread',
- );
- throw err;
- }
- }
+ const dbConnection = await DatabaseManager.getInstance().getGuildDb(guildId);
+ const repository = makeThreadRepository(dbConnection);
+
+ await handleDuplicateKeyError(
+ () => repository.create(data),
+ 'Thread',
+ data.id,
+ guildId,
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function createThread( | |
| guildId: Snowflake, | |
| data: IThread, | |
| ): Promise<void> { | |
| try { | |
| const dbConnection = | |
| await DatabaseManager.getInstance().getGuildDb(guildId); | |
| const repository = makeThreadRepository(dbConnection); | |
| await repository.create(data); | |
| logger.debug({ guildId, threadId: data.id }, 'Created thread'); | |
| } catch (err: any) { | |
| if (err.code === 11000) { | |
| logger.warn({ guildId, threadId: data.id }, 'Thread already exists'); | |
| } else { | |
| logger.error( | |
| { err, guildId, threadId: data.id }, | |
| 'Failed to create thread', | |
| ); | |
| throw err; | |
| } | |
| } | |
| } | |
| export async function createThread( | |
| guildId: Snowflake, | |
| data: IThread, | |
| ): Promise<void> { | |
| const dbConnection = await DatabaseManager.getInstance().getGuildDb(guildId); | |
| const repository = makeThreadRepository(dbConnection); | |
| await handleDuplicateKeyError( | |
| () => repository.create(data), | |
| 'Thread', | |
| data.id, | |
| guildId, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/activities/discord/gateway/persistence.activity.ts around lines 232 to
253, refactor the createThread function to replace the manual duplicate key
error check with the handleDuplicateKeyError helper function. Remove the if
(err.code === 11000) block and instead call handleDuplicateKeyError inside the
catch block, passing the error, guildId, data.id, and appropriate context to
maintain consistent error handling across create functions.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Style