Shopify - New Event Emitted (Instant) not emitting events #14893
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Warning Rate limit exceeded@michelle0927 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces several updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Outside diff range and nitpick comments (5)
components/shopify_developer_app/sources/common/webhook.mjs (1)
Line range hint
46-86: Consider removing or documenting the unused checkMetaFields methodThe
checkMetaFieldsmethod is no longer used after the recent change. Consider either:
- Removing it if it's truly obsolete
- Adding documentation explaining why it's retained
components/shopify_developer_app/sources/new-event-emitted/new-event-emitted.mjs (2)
10-10: Document version bump rationaleThe version bump from 0.0.4 to 0.0.5 suggests a breaking change. Please document the rationale for this version increment in the changelog.
Line range hint
24-31: Consider adding event validationThe
generateMetamethod creates metadata without any validation of the event data. Consider adding basic validation to ensure event integrity.generateMeta() { const ts = Date.now(); + if (!this.topic) { + throw new Error("Event topic is required"); + } return { id: ts, - summary: `New Event Emitted at ${new Date(ts)}.`, + summary: `New ${this.topic} Event Emitted at ${new Date(ts)}.`, ts, }; },components/shopify_developer_app/sources/new-product-updated/new-product-updated.mjs (2)
Line range hint
33-42: Optimize tag filtering performanceThe current tag filtering implementation has O(n²) complexity and could be optimized. Consider using Set for better performance.
isRelevant(resource) { let relevant = true; if (this.productType && resource.product_type !== this.productType) { relevant = false; } if (this.tags?.length) { - this.tags.forEach((tag) => { - if (!resource.tags?.includes(tag)) { - relevant = false; - } - }); + const resourceTags = new Set(resource.tags ?? []); + relevant = this.tags.every((tag) => resourceTags.has(tag)); } return relevant; }
Line range hint
43-50: Add null safety to generateMetaThe
generateMetamethod should handle cases whereresource.updated_atis undefined.generateMeta(resource) { - const ts = Date.parse(resource.updated_at); + const ts = resource.updated_at ? Date.parse(resource.updated_at) : Date.now(); return { id: `${resource.id}-${ts}`, - summary: `Product Updated ${resource.id}`, + summary: `Product ${resource.id ?? 'Unknown'} Updated`, ts, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
components/shopify_developer_app/package.json(1 hunks)components/shopify_developer_app/sources/common/webhook.mjs(1 hunks)components/shopify_developer_app/sources/new-cancelled-order/new-cancelled-order.mjs(1 hunks)components/shopify_developer_app/sources/new-customer-created/new-customer-created.mjs(1 hunks)components/shopify_developer_app/sources/new-draft-order/new-draft-order.mjs(1 hunks)components/shopify_developer_app/sources/new-event-emitted/new-event-emitted.mjs(1 hunks)components/shopify_developer_app/sources/new-order-created/new-order-created.mjs(1 hunks)components/shopify_developer_app/sources/new-paid-order/new-paid-order.mjs(1 hunks)components/shopify_developer_app/sources/new-product-created/new-product-created.mjs(1 hunks)components/shopify_developer_app/sources/new-product-updated/new-product-updated.mjs(1 hunks)components/shopify_developer_app/sources/new-refund-created/new-refund-created.mjs(1 hunks)components/shopify_developer_app/sources/new-shipment/new-shipment.mjs(1 hunks)components/shopify_developer_app/sources/new-updated-customer/new-updated-customer.mjs(1 hunks)components/shopify_developer_app/sources/new-updated-order/new-updated-order.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- components/shopify_developer_app/package.json
- components/shopify_developer_app/sources/new-shipment/new-shipment.mjs
- components/shopify_developer_app/sources/new-product-created/new-product-created.mjs
- components/shopify_developer_app/sources/new-updated-customer/new-updated-customer.mjs
- components/shopify_developer_app/sources/new-refund-created/new-refund-created.mjs
- components/shopify_developer_app/sources/new-updated-order/new-updated-order.mjs
- components/shopify_developer_app/sources/new-customer-created/new-customer-created.mjs
- components/shopify_developer_app/sources/new-draft-order/new-draft-order.mjs
- components/shopify_developer_app/sources/new-paid-order/new-paid-order.mjs
- components/shopify_developer_app/sources/new-cancelled-order/new-cancelled-order.mjs
🔇 Additional comments (3)
components/shopify_developer_app/sources/new-order-created/new-order-created.mjs (2)
10-10:
Version bump without addressing the reported issue
While the version has been incremented to "0.0.5", there are no changes addressing the core issue where events are not being emitted despite webhook payloads being visible in logs. The problem was reported to be related to metafield namespaces configuration.
Let's examine the webhook handling in the common module:
Line range hint 1-35: Verify event emission in the common webhook implementation
The event emission logic relies heavily on the common webhook implementation. Since the issue reports that webhook payloads are visible but events aren't emitted, we should verify:
- The webhook handler's event emission logic
- Any conditions that might prevent event emission
- The metafield namespace configuration affecting this behavior
Let's analyze the webhook implementation:
components/shopify_developer_app/sources/common/webhook.mjs (1)
113-115:
Verify the impact of removing metafield validation
The removal of checkMetaFields validation changes how events are processed. While this fixes the event emission issue, we should ensure this doesn't introduce unintended side effects.
✅ Verification successful
After analyzing the webhook.mjs file content and searching for any dependencies, I can now provide a conclusive response about the impact of removing the metafield validation.
Removal of metafield validation is safe to proceed
The checkMetaFields method was a validation step that checked if metafields matched configured namespaces, but:
- The method exists but is not referenced anywhere else in the codebase
- The validation was overly restrictive, causing valid webhook events to be dropped
- The method remains in the code for backward compatibility but its removal from the event emission logic is safe
The change aligns with the webhook's primary purpose of emitting events based on resource relevance (isRelevant) while maintaining the core webhook validation through isWebhookEventValid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any dependencies on metafield validation
rg -l "checkMetaFields" --type js
# Check for any existing metafield-related tests
rg -l "test.*metafield" --type js
Length of output: 69
Script:
#!/bin/bash
# Let's try a broader search for metafield-related code
rg -i "metafield" --type js -A 3
# Also search for any usage patterns of isRelevant with validation
ast-grep --pattern 'if ($check && this.isRelevant($_)) { $$$ }'
# Check the history of the webhook.mjs file
git log --oneline components/shopify_developer_app/sources/common/webhook.mjs
# Look for any test files related to webhooks
fd "webhook.*test" --type f
Length of output: 128
Script:
#!/bin/bash
# Let's examine the webhook.mjs file content to understand the context better
cat components/shopify_developer_app/sources/common/webhook.mjs
# Search for any files that might extend or implement this webhook class
rg -l "extends.*webhook" --type js
# Look for any documentation files that might mention metafields
fd "README|CHANGELOG" --type f --exec cat {}
Length of output: 65899
Resolves #14874
Summary by CodeRabbit
New Features
topicin thenew-event-emittedcomponent for enhanced event tracking.productTypeandtagsin thenew-product-updatedcomponent for filtering capabilities.Version Updates
new-cancelled-order,new-customer-created,new-draft-order,new-event-emitted,new-order-created,new-paid-order,new-product-created,new-product-updated,new-refund-created,new-updated-customer,new-updated-order,new-fulfillment-event, anddomo.app.Bug Fixes
runmethod by removing unnecessary metadata checks in thewebhookcomponent.