fix(api-service,dashboard): fix attachment crash, context race condition, and CodeMirror decoration error#10812
Open
cursor[bot] wants to merge 2 commits intonextfrom
Open
fix(api-service,dashboard): fix attachment crash, context race condition, and CodeMirror decoration error#10812cursor[bot] wants to merge 2 commits intonextfrom
cursor[bot] wants to merge 2 commits intonextfrom
Conversation
… CodeMirror decoration error - API-PC: Filter out attachments with undefined file before Buffer.from() call - API-PQ: Catch MongoDB E11000 duplicate key error and return 409 ConflictException - DASHBOARD-2A7: Skip variable/translation decorations that span line breaks Fixes API-PC Fixes API-PQ Fixes DASHBOARD-2A7 Co-authored-by: Dima Grossman <dima@grossman.io>
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
Contributor
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
📝 WalkthroughWalkthroughThe PR adds defensive programming measures across API and dashboard services: wrapping context creation in error handling to translate duplicate-key failures to conflict exceptions, filtering null attachments before processing, and preventing UI decorators from rendering multiline regex matches in both translation and variable plugins. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/dashboard/src/components/primitives/variable-plugin/plugin-view.ts (1)
63-65: Correct guard against multiline replace-decoration crash.Skipping matches that span newlines prevents CodeMirror's "Decorations that replace line breaks may not be specified via plugins" error. One minor optimization: moving this check immediately after computing
match[0](e.g., right after Line 52 or beforeparseVariable) avoids the unnecessaryparseVariablecall for multiline matches. Not a blocker.♻️ Optional micro-optimization
while ((match = regex.exec(content)) !== null) { + if (match[0].includes('\n')) { + continue; + } + const parsedVariable = parseVariable(match[0]); if (!parsedVariable) { continue; } const { fullLiquidExpression, name, filtersArray } = parsedVariable; const start = match.index; const end = start + match[0].length; - if (match[0].includes('\n')) { - continue; - } -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/primitives/variable-plugin/plugin-view.ts` around lines 63 - 65, Move the multiline-guard so we check match[0].includes('\n') immediately after computing match[0] and before calling parseVariable, so multiline matches are skipped without invoking parseVariable; update the loop that iterates matches (the code using match[0] and parseVariable) to continue early for newline-containing matches to avoid CodeMirror's "replace line breaks" decoration crash.apps/api/src/app/contexts/usecases/create-context/create-context.usecase.ts (1)
19-36: Optional: de-duplicate the conflict message.The ConflictException message is identical on Line 19 and Line 36. Extracting it into a small helper or local constant avoids the two sites drifting apart in future edits.
♻️ Proposed refactor
async execute(command: CreateContextCommand): Promise<ContextEntity> { + const conflictMessage = `Context with type '${command.type}' and id '${command.id}' already exists`; + const existingContext = await this.contextRepository.findOne({ _environmentId: command.environmentId, _organizationId: command.organizationId, type: command.type, id: command.id, }); if (existingContext) { - throw new ConflictException(`Context with type '${command.type}' and id '${command.id}' already exists`); + throw new ConflictException(conflictMessage); } try { return await this.contextRepository.create({ _environmentId: command.environmentId, _organizationId: command.organizationId, type: command.type, id: command.id, key: createContextKey(command.type, command.id), data: command.data || {}, }); } catch (error) { const isDuplicateKeyError = error && typeof error === 'object' && 'code' in error && error.code === ErrorCodesEnum.DUPLICATE_KEY; if (isDuplicateKeyError) { - throw new ConflictException(`Context with type '${command.type}' and id '${command.id}' already exists`); + throw new ConflictException(conflictMessage); } throw error; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/contexts/usecases/create-context/create-context.usecase.ts` around lines 19 - 36, The ConflictException message is duplicated; inside the execute method (the CreateContextUsecase surrounding the early throw and the catch block that checks ErrorCodesEnum.DUPLICATE_KEY) extract the string into a single constant or small helper (e.g., const conflictMessage = `Context with type '${command.type}' and id '${command.id}' already exists` or a getConflictMessage(command) function) declared before the first throw, then replace both uses of the inline message (the initial throw and the throw in the catch) with that constant/helper so the text is maintained in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/app/contexts/usecases/create-context/create-context.usecase.ts`:
- Around line 19-36: The ConflictException message is duplicated; inside the
execute method (the CreateContextUsecase surrounding the early throw and the
catch block that checks ErrorCodesEnum.DUPLICATE_KEY) extract the string into a
single constant or small helper (e.g., const conflictMessage = `Context with
type '${command.type}' and id '${command.id}' already exists` or a
getConflictMessage(command) function) declared before the first throw, then
replace both uses of the inline message (the initial throw and the throw in the
catch) with that constant/helper so the text is maintained in one place.
In `@apps/dashboard/src/components/primitives/variable-plugin/plugin-view.ts`:
- Around line 63-65: Move the multiline-guard so we check
match[0].includes('\n') immediately after computing match[0] and before calling
parseVariable, so multiline matches are skipped without invoking parseVariable;
update the loop that iterates matches (the code using match[0] and
parseVariable) to continue early for newline-containing matches to avoid
CodeMirror's "replace line breaks" decoration crash.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2abc0026-b2db-44af-9b8f-26acabc81702
📒 Files selected for processing (4)
apps/api/src/app/contexts/usecases/create-context/create-context.usecase.tsapps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.tsapps/dashboard/src/components/primitives/translation-plugin/plugin-view.tsapps/dashboard/src/components/primitives/variable-plugin/plugin-view.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three production Sentry errors affecting the API and Dashboard.
1. API-PC:
Buffer.from(undefined)crash in attachment parsing (220 events, 1 user)Root cause:
POST /v1/events/triggercallsmodifyAttachmentswhich maps overcommand.payload.attachmentsand callsBuffer.from(attachment.file, 'base64'). When a caller sends an attachment object without afileproperty,attachment.fileisundefinedandBuffer.fromthrows aTypeError.Fix: Filter out attachments with
null/undefinedfilebefore the.map()call. Attachments without file data are silently skipped rather than crashing the entire trigger request.2. API-PQ: MongoDB E11000 duplicate key error on context creation (102,004 events, 23 users)
Root cause:
POST /v2/contextsdoes afindOnecheck thencreate(check-then-act). Under concurrent requests for the same context, both pass thefindOnecheck and race tocreate, causing one to hit the unique index constraint. The rawMongoServerErrorbubbles up as a 500 instead of the intended 409.Fix: Wrap the
createcall in a try/catch that detectsE11000(duplicate key) errors and converts them toConflictException(409), matching the existing behavior for the non-racy path. This mirrors the pattern already used inContextRepository.findOrCreateContext.3. DASHBOARD-2A7: CodeMirror decoration crash on multiline variables (50 events, 2 users)
Root cause: The variable regex
{{([^{}]+)}}allows newlines in matches. When a variable expression like{{\nsubscriber.name\n}}spans multiple lines,Decoration.replace().range(start, end)crosses a line boundary. CodeMirror throws: "Decorations that replace line breaks may not be specified via plugins".Fix: Skip creating replace decorations when the matched text contains a newline, in both the variable plugin and translation plugin
createDecorationsmethods. The translation plugin'sparseTranslationalready had a newline guard, but the check at the decoration level provides defense-in-depth for both plugins.What changed
This PR fixes three production bugs (220–102k events) affecting the API and Dashboard. The API attachment parsing was crashing when encountering attachments with undefined file data. Context creation had a race condition where concurrent requests could both bypass duplicate-key checks and cause MongoDB E11000 errors. CodeMirror decorations were throwing errors on multiline variable matches. Fixes: filter null attachments before conversion, wrap context creation in try/catch to translate duplicate-key errors to 409 responses, skip creating decorations for matches containing newlines.
Affected areas
Testing
No information provided about new tests in the PR; verification would consist of confirming these Sentry error frequencies decline in production monitoring.