-
Notifications
You must be signed in to change notification settings - Fork 5
fix: idempotency in dreamsync adapter #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughConsentService now wraps consent processing in an operation-scoped context with a unique operationId. Additionally, webhook handlers in subscriber.ts add guards to skip processing unless originating from ConsentService, preventing duplicate operations. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ConsentService
participant OperationContext
participant Subscriber
participant WebhookHandler
Caller->>ConsentService: processConsentResponse()
ConsentService->>OperationContext: withOperationContext('ConsentService', operationId)
OperationContext->>ConsentService: execute processing
ConsentService->>Subscriber: emit change event
Subscriber->>WebhookHandler: handleChange()
WebhookHandler->>WebhookHandler: shouldProcessWebhook(id, table)?
alt originates from ConsentService
WebhookHandler->>WebhookHandler: proceed with webhook
else
WebhookHandler->>WebhookHandler: early return (skip)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow consistent, straightforward patterns: operation context wrapping in one service and guard clause additions across webhook handlers. No complex business logic or structural changes; primarily contextual and filtering enhancements. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
platforms/dreamsync-api/src/services/ConsentService.ts (1)
24-27: Consider using a more robust operation ID generation strategy.The current approach combines
Date.now()withMath.random(), which has a small collision risk if multiple consent operations are processed in the same millisecond. Consider using a UUID library (e.g.,uuid.v4()) for guaranteed uniqueness.Apply this diff to use a UUID library:
+import { v4 as uuidv4 } from 'uuid'; + async processConsentResponse(messageText: string, senderId: string, groupId: string): Promise<void> { // Generate unique operation ID for this consent processing - const operationId = `consent-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const operationId = `consent-${uuidv4()}`;platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts (1)
424-429: Consider documenting the idempotency strategy.The webhook filtering mechanism is a critical part of the idempotency solution, but it's not immediately clear from the code why certain entities are "protected" or how this prevents duplicate operations. Consider adding comprehensive documentation explaining:
- Which entities are protected and why
- How the operation context tracks ConsentService operations
- What happens to webhooks from other services
Add a comment block above this method:
/** * Send webhook for group entity * * NOTE: This method is part of the idempotency solution. It only processes * webhooks for groups created/updated by ConsentService to prevent duplicate * operations when groups are created as a result of webhook processing. * * Groups created outside ConsentService (e.g., via API) should be processed * by a different mechanism. */ private async sendGroupWebhook(data: any): Promise<void> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/dreamsync-api/src/services/ConsentService.ts(3 hunks)platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: lint
🔇 Additional comments (2)
platforms/dreamsync-api/src/services/ConsentService.ts (1)
27-161: Unable to verify withOperationContext error handling—implementation file not found in codebase.The review comment requests verification that
withOperationContextproperly handles async errors and doesn't interfere with error propagation. However, the actual implementation file cannot be located:
- ConsentService.ts imports
withOperationContextfrom"../context/OperationContext"- The expected file path
platforms/dreamsync-api/src/context/OperationContext.tsdoes not exist- No definition or export of
withOperationContextwas found in the codebaseThis indicates either the import path is incorrect, the file is in an unexpected location, or the source files are not available for inspection.
platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts (1)
354-359: shouldProcessWebhook function implementation is missing from the repository.The function is imported in
platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts(line 13) from../../context/OperationContext, but this file does not exist.The review comment's concern about entity name variations is valid: the function is called with both singular forms (e.g.,
junctionInfo.entityat line 355, which resolves to "Group", "User", etc.) and plural forms (e.g.,"groups"at line 425, andtableNameat line 258 from database metadata).Before this code can be merged, you must:
- Create the missing
platforms/dreamsync-api/src/context/OperationContext.tsfile- Ensure
shouldProcessWebhookimplementation normalizes entity names or explicitly handles both singular and plural forms consistently
| import { Message } from "../database/entities/Message"; | ||
| import { UserService } from "./UserService"; | ||
| import { GroupService } from "./GroupService"; | ||
| import { withOperationContext } from "../context/OperationContext"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that the OperationContext module exists.
The import references ../context/OperationContext, but this module is not included in the files under review. Ensure that withOperationContext is implemented and exported from this module.
Run the following script to verify the module exists and exports the required function:
🏁 Script executed:
#!/bin/bash
# Description: Verify OperationContext module exists and exports withOperationContext
# Check if the file exists
if [ -f "platforms/dreamsync-api/src/context/OperationContext.ts" ]; then
echo "✅ OperationContext.ts exists"
# Check for withOperationContext export
rg -n "export.*withOperationContext" platforms/dreamsync-api/src/context/
else
echo "❌ OperationContext.ts not found"
fiLength of output: 31
The web search results are not related to the specific dreamsync-api project. Let me search the codebase to find if OperationContext exists anywhere and to understand the structure better.
🏁 Script executed:
#!/bin/bash
# Search for any OperationContext file in the codebase
echo "=== Searching for OperationContext files ==="
fd -e ts -e tsx "OperationContext" platforms/dreamsync-api/
echo "=== Checking context directory contents ==="
if [ -d "platforms/dreamsync-api/src/context" ]; then
echo "✅ context directory exists"
ls -la platforms/dreamsync-api/src/context/
else
echo "❌ context directory does not exist"
fi
echo "=== Searching for withOperationContext references in codebase ==="
rg -n "withOperationContext" platforms/dreamsync-api/src/ -A 2 -B 2Length of output: 1232
The OperationContext module is missing and will cause a runtime import error.
The import on line 8 references ../context/OperationContext, but neither the context directory nor the OperationContext.ts file exist in the codebase. The withOperationContext function is called on line 27, but it has no implementation anywhere in the repository. Create the missing module with the required export, or remove/replace this import if it's not needed.
🤖 Prompt for AI Agents
In platforms/dreamsync-api/src/services/ConsentService.ts around line 8, the
import ../context/OperationContext is missing and will cause a runtime error;
either create that module at
platforms/dreamsync-api/src/context/OperationContext.ts (so the relative import
resolves) or remove/replace the import if unused. If creating the module, export
a typed withOperationContext helper that accepts an async operation (e.g.
function or lambda) and executes it within the operation context lifecycle
(initialize any context, invoke the operation, handle/propagate errors, and
clean up), and ensure the export name matches exactly (export function
withOperationContext...). If you prefer to remove it, delete the import and any
calls to withOperationContext (or replace them with direct invocation of the
operation) to avoid unresolved references.
| import path from "path"; | ||
| import dotenv from "dotenv"; | ||
| import { AppDataSource } from "../../database/data-source"; | ||
| import { shouldProcessWebhook } from "../../context/OperationContext"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that shouldProcessWebhook is implemented and exported.
The import references shouldProcessWebhook from ../../context/OperationContext, but this function is not included in the files under review. Ensure that it's properly implemented and handles both singular and plural entity names.
Run the following script to verify the function exists and check its signature:
🏁 Script executed:
#!/bin/bash
# Description: Verify shouldProcessWebhook function exists and check its implementation
# Check for shouldProcessWebhook export
rg -n -A 15 "export.*function shouldProcessWebhook" platforms/dreamsync-api/src/context/
# Also check if it handles entity name variations (singular vs plural)
rg -C 3 "shouldProcessWebhook" platforms/dreamsync-api/src/context/Length of output: 153
I need to search for the actual locations of the files and functions. Let me explore the codebase structure:The web search doesn't provide information about the specific codebase structure. Let me search the codebase directly to find the correct file locations:
🏁 Script executed:
#!/bin/bash
# Find OperationContext file location
echo "=== Searching for OperationContext file ==="
fd -type f "OperationContext" platforms/
echo ""
echo "=== Searching for shouldProcessWebhook definition ==="
rg -n "shouldProcessWebhook" platforms/ -g "!node_modules"
echo ""
echo "=== Searching for subscriber.ts to verify path ==="
fd -type f "subscriber.ts" platforms/Length of output: 1022
Let me search for the OperationContext file with corrected commands:
🏁 Script executed:
#!/bin/bash
# Find OperationContext file with correct syntax
echo "=== Searching for OperationContext file ==="
find platforms/ -name "*OperationContext*" 2>/dev/null
echo ""
echo "=== Searching for export of shouldProcessWebhook ==="
rg -n "export.*shouldProcessWebhook" platforms/
echo ""
echo "=== Broader search for shouldProcessWebhook definition ==="
rg -n "function shouldProcessWebhook|const shouldProcessWebhook" platforms/ -A 5
echo ""
echo "=== List src/context directory contents ==="
find platforms/dreamsync-api/src/context -type f 2>/dev/null | head -20Length of output: 202
Let me verify the directory structure more broadly:
🏁 Script executed:
#!/bin/bash
# Check src directory structure
echo "=== Contents of platforms/dreamsync-api/src ==="
ls -la platforms/dreamsync-api/src/ 2>/dev/null
echo ""
echo "=== All directories in src ==="
find platforms/dreamsync-api/src -mindepth 1 -maxdepth 2 -type d 2>/dev/null | sort
echo ""
echo "=== Verify the import path in subscriber.ts ==="
head -20 platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts
echo ""
echo "=== Check if any context-related files exist anywhere ==="
find platforms/dreamsync-api -name "*context*" -o -name "*Context*" 2>/dev/nullLength of output: 1964
The import references a non-existent module and undefined function - must be implemented.
Verification confirms that shouldProcessWebhook is not implemented or exported anywhere in the codebase. The context/OperationContext module referenced at platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts:13 does not exist. The function is actively used at lines 258, 355, and 425 in the same file, but has no implementation. This import will fail at runtime.
The context/ directory does not exist in platforms/dreamsync-api/src/. The directory structure contains: controllers/, database/, middleware/, services/, types/, utils/, and web3adapter/ only.
🤖 Prompt for AI Agents
In platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts around line
13, the import of shouldProcessWebhook from "../../context/OperationContext"
points to a non-existent module and the function is used later at lines 258,
355, and 425; create the missing module or move the function to an existing
place and export it. Add a new file
platforms/dreamsync-api/src/context/OperationContext.ts (and create the context/
folder) that exports a properly typed shouldProcessWebhook function (accepting
the same parameters used in subscriber.ts and returning a boolean or
Promise<boolean>), implement its logic according to how the subscriber expects
to decide whether to process a webhook, and update or add index exports if your
project uses barrel files; alternatively, if there is an appropriate utils or
service file, implement and export shouldProcessWebhook there and update the
import in subscriber.ts to the correct relative path.
| // Check if this operation should be processed (only ConsentService operations for groups/messages) | ||
| if (!shouldProcessWebhook(entity?.id, tableName)) { | ||
| console.log(`⏭️ Skipping webhook for ${tableName}:${entity?.id} - not from ConsentService (protected entity)`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that blocking non-ConsentService webhooks doesn't break existing functionality.
This guard blocks ALL webhook processing for protected entities (groups/messages) unless they originate from ConsentService. This is a significant behavioral change that could break other parts of the system that rely on webhooks for groups or messages.
Run the following script to identify other code paths that might be affected:
🏁 Script executed:
#!/bin/bash
# Description: Find other services or code that might create/update groups or messages
# Search for group creation/updates outside ConsentService
rg -n "createGroup|updateGroup|save.*group" platforms/dreamsync-api/src --type ts -g '!ConsentService.ts' -g '!subscriber.ts' -C 2
# Search for message creation outside ConsentService
rg -n "create.*message|save.*message" platforms/dreamsync-api/src --type ts -g '!ConsentService.ts' -g '!subscriber.ts' -C 2Length of output: 17274
I need to examine the shouldProcessWebhook function and understand how it determines entity origin:
Wait, I need the repository. Let me correct that:
🏁 Script executed:
#!/bin/bash
# Find shouldProcessWebhook function definition
rg -n "shouldProcessWebhook" platforms/dreamsync-api/src --type ts -A 10
# Also check imports/usage in subscriber.ts
rg -n "import\|shouldProcessWebhook" platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts -B 5 -A 5Length of output: 5102
🏁 Script executed:
#!/bin/bash
# Find shouldProcessWebhook function definition
rg -n "shouldProcessWebhook" platforms/dreamsync-api/src --type ts -A 10
# Also check imports/usage in subscriber.ts
rg -n "import\|shouldProcessWebhook" platforms/dreamsync-api/src/web3adapter/watchers/subscriber.ts -B 5 -A 5Length of output: 5102
---
🏁 Script executed:
```shell
#!/bin/bash
# Find OperationContext file
fd -n "OperationContext" platforms/dreamsync-api/src --type f
# Look for the shouldProcessWebhook function implementation
rg -n "export.*shouldProcessWebhook|function shouldProcessWebhook|const shouldProcessWebhook" platforms/dreamsync-api/src --type ts -A 15
Length of output: 230
---
🏁 Script executed:
```shell
#!/bin/bash
# Find OperationContext file - corrected syntax
fd "OperationContext" platforms/dreamsync-api/src --type f
# Search for shouldProcessWebhook function definition
rg "export.*shouldProcessWebhook|function shouldProcessWebhook|const shouldProcessWebhook" platforms/dreamsync-api/src --type ts -A 15
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# List context directory if it exists
ls -la platforms/dreamsync-api/src/context/ 2>/dev/null || echo "Context directory not found"
# Broad search for shouldProcessWebhook
rg "shouldProcessWebhook" platforms/dreamsync-api/src --type ts
# Search for OperationContext anywhere
rg "OperationContext" platforms/dreamsync-api/src --type ts
Length of output: 935
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for withOperationContext definition
rg "withOperationContext|export.*context" platforms/dreamsync-api/src --type ts -B 2 -A 8
# Check if context directory exists at different level
find platforms/dreamsync-api -type d -name context 2>/dev/null
# List all files that contain context-related code
rg "OperationContext|withOperationContext" platforms/dreamsync-api --type ts -l
Length of output: 2291
The code references a non-existent module and will fail at runtime.
The imports in subscriber.ts (line 13) and ConsentService.ts attempt to load ../context/OperationContext, but this module doesn't exist in the codebase. The directory platforms/dreamsync-api/src/context/ is not present, and neither shouldProcessWebhook nor withOperationContext are defined anywhere.
This means:
- The webhook guard at
subscriber.ts:257-261calls an undefined function that will throwCannot find moduleerror at runtime - The feature is incomplete and cannot execute
- The review comment's concern about breaking existing functionality is superseded by this critical issue
Action required: Create the missing OperationContext module with proper implementations of shouldProcessWebhook() and withOperationContext(), or remove these non-functional references entirely.
Description of change
Issue Number
closes #375
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit