Repository navigation
Conversation
…no listener
The visual builder registers the request-discussion-highlights listener
inside DiscussionPage, which Venus only mounts while the Discussions panel
is open. With the panel closed, which is the default, the send rejects with
NO_REQUEST_LISTENER_FOUND and nothing handles it. Next.js dev registers a
global unhandledrejection handler and renders the plain {code, message}
rejection as a runtime error overlay titled "[object Object]".
Handle the rejection at both send sites, since an absent receiver is the
expected state for this event rather than a failure.
Both branches of the isSSR check sent the same event, so the send is hoisted
out of the branch instead of being duplicated.
The spec mocked send as vi.fn(), which returns undefined rather than a
promise. Tests only passed because two later tests set mockResolvedValue and
vi.clearAllMocks does not reset implementations, so every test after them
inherited a promise. The base mock now matches the real contract.
Co-Authored-By: Claude <noreply@anthropic.com>
…a missing listener Swallowing every rejection hid genuine problems. NO_ACK_RECEIVED, WINDOW_CLOSED and CODE_RETURNED_ERROR would have been discarded as quietly as the expected missing listener. ignoreMissingListener returns silently only for NO_REQUEST_LISTENER_FOUND, which is the normal state while the Discussions panel is closed, and warns through PublicLogger for anything else. PublicLogger is already used across the SDK and is silent under NODE_ENV=test, so this adds no console noise in the common case. The helper lives in its own module because the spec mocks the visualBuilderPostMessage module wholesale; keeping it separate means tests exercise the real function rather than a mock. Co-Authored-By: Claude <noreply@anthropic.com>
…ver the observer send Review found that the helper's doc comment and its tests described rejection shapes the library does not produce. Verified against @contentstack/advanced-post-message 0.0.5: only the missing-listener reply carries a code. The closed-window path rejects with an uncoded Error, the no-ack timeout rejects with a bare string, and CODE_RETURNED_ERROR is logged on the receiving side and never sent back, so the sender never sees it. The handler already did the right thing for all of these, since an uncoded rejection warns. Only the doc comment and the tests were wrong, so the code check stays as the single rule and the comment now says so. Matching the no-ack case too would mean matching on a message string, and a receiver that never acks is worth a warning anyway. Also from review: - type the event parameter as VisualBuilderPostMessageEvents instead of string - restate the send mock contract in beforeEach so the spec no longer depends on implementations leaking between tests, and drop the hand-restore - add a spec for the debounced observer send, which had no coverage. The suite stubs MutationObserver globally in vitest.setup.ts, so the spec captures the observer callback and drives it directly. Co-Authored-By: Claude <noreply@anthropic.com>
Review pointed out that asserting `.catch` was called proves a handler was
attached, not which one: swapping ignoreMissingListener for `() => {}` kept
the tests green.
Asserting PublicLogger.warn stays silent on the missing-listener code does
not close that gap either, since a no-op catch is also silent. Verified by
mutation: with `() => {}` at both call sites, the silent-case assertions
still pass. What fails is a rejection the helper is supposed to warn about,
so each spec now has that case, using the uncoded string the library's
no-ack timeout actually rejects with.
Also from review: import VisualBuilderPostMessageEvents as a type in
postMessageErrors.ts, since it is only used in a type position.
Co-Authored-By: Claude <noreply@anthropic.com>
…y spy lifecycles Review found that every warn-branch assertion read only the message the helper builds from the event name, so dropping the error argument from PublicLogger.warn kept all of them green while the warning lost the part that says what failed. All four sites now assert the second argument. Verified by mutation: removing that argument fails all four. Also from review: - capture MutationObserver in beforeEach rather than at module scope. The setup file installs its stub in beforeAll, so the module-level read took jsdom's native implementation and afterEach restored the wrong one. - restore spies in the SSR block's afterEach. clearAllMocks leaves the implementation in place, so PublicLogger.warn stayed stubbed for the rest of the file once these tests had run. - assert the rejected value instead of toBeTruthy, which held regardless of what the code did. Co-Authored-By: Claude <noreply@anthropic.com>
…nhandled-rejection fix(visual-builder): handle rejection when discussion highlights has no listener
Co-Authored-By: Claude <noreply@anthropic.com>
Updated codeowners
Co-Authored-By: Claude <noreply@anthropic.com>
release: v4.5.3 (sync main into develop_v4)
release: v4.5.3 (Develop v4 -> stage v4)
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
hitesh-shetty-cstk
left a comment
hitesh-shetty-cstk
left a comment
There was a problem hiding this comment.
Automated review
What this changes: A patch release cut of v4.5.3. The substance is one fix: both sites that send REQUEST_DISCUSSION_HIGHLIGHTS to the Visual Builder now attach a rejection handler, so an absent receiver no longer surfaces as an unhandled promise rejection. A new ignoreMissingListener helper returns silently for the library's missing-listener reply and warns through PublicLogger for anything else. The rest is release mechanics: version bump in package.json and package-lock.json, regenerated CHANGELOG, and the README CDN pin.
Business impact: None identified beyond the fix itself. I checked the one restructure that could have changed behaviour: the REQUEST_DISCUSSION_HIGHLIGHTS send in useVariantFieldsPostMessageEvent moved out of both branches of the isSSR conditional to a single call after it. Both branches sent it before, in the same position relative to addVariantFieldClass and updateVariantClasses, so when and how often it fires is unchanged. The CSR double send, once directly and once from the debounced observer, predates this change and the new comment now says so.
Security: Nothing beyond what the scanners cover. Snyk reported clean on this head. The new warning path writes the raw rejection to the console, but this code runs in the Visual Builder context rather than on visitor traffic, and the rejection carries postMessage transport detail rather than entry content.
Flow
sequenceDiagram
participant VE as useVariantFieldsPostMessageEvent
participant OBS as requestDiscussionHighlights
participant PM as visualBuilderPostMessage
participant VB as Visual Builder parent window
participant ERR as ignoreMissingListener
participant LOG as PublicLogger
VE->>PM: send REQUEST_DISCUSSION_HIGHLIGHTS
OBS->>PM: send REQUEST_DISCUSSION_HIGHLIGHTS
PM->>VB: postMessage
alt Discussions panel mounted
VB-->>PM: ack, resolves
else no listener mounted
VB-->>PM: reject with code NO_REQUEST_LISTENER_FOUND
PM->>ERR: catch
ERR-->>ERR: return without logging
else closed window or no ack
PM-->>ERR: catch
ERR->>LOG: warn with event name and rejection
end
Note over PM,LOG: changed here
Findings: 0 blocker, 1 should fix, 1 nit. Both are inline.
On the test changes, which I looked at closely because assertions that only appear to guard a fix are easy to write here: the suite does hold. The catchSpy assertions are coupled to the .catch shape, and the code comment is candid that they would not survive a refactor to try/catch. Independently of that, the two "warns through the helper" cases run the real ignoreMissingListener and would fail if the handler were dropped, so the fix is guarded either way. The added mockResolvedValue(undefined) restatements in both beforeEach blocks are needed rather than redundant, since the new tests swap in a rejecting promise that clearAllMocks does not undo, and the SSR block is the last one in the file so nothing inherits it. The MutationObserver capture is taken in beforeEach for the reason its comment gives: the global stub is installed in beforeAll, so a module-level read would restore the wrong implementation.
Reviewer candidates: @csAyushDubey authored 7 of the last 43 commits across useVariantsPostMessageEvent.ts and its spec. @Deepak-Kharah authored 4 of the last 30 on useVariantsPostMessageEvent.ts. Three of the five largest files in this diff are new, so the history is thinner than the file count suggests. @kirtesh-cstk has already reviewed.
Not covered: I did not run the tests, the build, or the app. @contentstack/advanced-post-message is not installed in the checkout I read from, so the library's actual rejection shapes and the NO_REQUEST_LISTENER_FOUND value are unverified, which is the subject of the inline comment on the helper. I also did not verify the post-merge publish and CDN steps. Separately, many other send call sites across the SDK still have no rejection handler; that is pre-existing and outside the scope of this release, noted only so the narrow fix is not mistaken for a general one.
Automated review by Claude Code. A human review is still required.
Generated by Claude Code
Live Preview SDK v4.5.3
Summary
Patch release. Sending discussion-highlight updates from the Visual Builder no longer raises an unhandled promise rejection when no listener is mounted on the other side.
What's changed
Fixes
ignoreMissingListenerhelper insrc/visualBuilder/utils/postMessageErrors.tsswallows the library's missing-listener rejection and logs a console warning for any other send failure, such as a closed window or a receiver that never acks. Both send sites use it:useVariantsPostMessageEventanduseRecalculateVariantDataCSLPValues.Tests
Chores
4.5.3; CHANGELOG regenerated viaauto-changelog.@contentstack/live-preview-utils@4.5.3.Post-merge checklist
stage_v4tomain)v4.5.3frommain(tag created via the Release UI), which triggers thenpm-publishworkflow@contentstack/live-preview-utils@4.5.3is published to npmhttps://esm.sh/@contentstack/live-preview-utils@4.5.3resolves🤖 Generated with Claude Code