diff --git a/src/visualBuilder/eventManager/__test__/useRecalculateVariantDataCSLPValues.spec.ts b/src/visualBuilder/eventManager/__test__/useRecalculateVariantDataCSLPValues.spec.ts new file mode 100644 index 00000000..8364f7c3 --- /dev/null +++ b/src/visualBuilder/eventManager/__test__/useRecalculateVariantDataCSLPValues.spec.ts @@ -0,0 +1,129 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"; + +// Identity debounce so the observer's request fires within the test. +vi.mock("lodash-es", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, debounce: vi.fn((fn: any) => fn) }; +}); + +vi.mock("../../../visualBuilder/utils/visualBuilderPostMessage", () => ({ + default: { on: vi.fn(), send: vi.fn().mockResolvedValue(undefined) }, +})); + +vi.mock("../../../visualBuilder", () => ({ + VisualBuilder: { + VisualBuilderGlobalState: { + value: { variant: null, highlightVariantFields: false }, + }, + }, +})); + +import { updateVariantClasses } from "../useRecalculateVariantDataCSLPValues"; +import { VisualBuilderPostMessageEvents } from "../../utils/types/postMessage.types"; +import visualBuilderPostMessage from "../../../visualBuilder/utils/visualBuilderPostMessage"; +import * as cslpdata from "../../../cslp/cslpdata"; +import { PublicLogger } from "../../../logger/logger"; + +const send = (visualBuilderPostMessage as any).send; + +// vitest.setup.ts swaps MutationObserver for a stub that drops the callback, so +// the real observer never fires here. Capture the callback and drive it instead. +let observerCallbacks: MutationCallback[] = []; +// Captured in beforeEach, not at module scope: vitest.setup.ts installs its +// stub in beforeAll, so a module-level read would grab jsdom's native one and +// afterEach would restore the wrong implementation. +let installedMutationObserver: typeof MutationObserver; + +class CapturingMutationObserver { + observe = vi.fn(); + disconnect = vi.fn(); + takeRecords = vi.fn((): MutationRecord[] => []); + constructor(callback: MutationCallback) { + observerCallbacks.push(callback); + } +} + +const attributeMutation = { + type: "attributes", + attributeName: "data-cslp", + addedNodes: [] as unknown as NodeList, +} as unknown as MutationRecord; + +describe("requestDiscussionHighlights via the CSLP mutation observer", () => { + beforeEach(() => { + vi.clearAllMocks(); + observerCallbacks = []; + installedMutationObserver = global.MutationObserver; + global.MutationObserver = + CapturingMutationObserver as unknown as typeof MutationObserver; + send.mockResolvedValue(undefined); + vi.spyOn(cslpdata, "isValidCslp").mockReturnValue(true); + document.body.innerHTML = `

hi

`; + }); + + afterEach(() => { + global.MutationObserver = installedMutationObserver; + document.body.innerHTML = ""; + vi.restoreAllMocks(); + }); + + function fireMutation() { + updateVariantClasses(); + expect(observerCallbacks).toHaveLength(1); + observerCallbacks[0]([attributeMutation], {} as MutationObserver); + } + + it("requests discussion highlights when a data-cslp attribute changes", () => { + fireMutation(); + + expect(send).toHaveBeenCalledWith( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + }); + + // The bug this guards: the Discussions panel is closed on most CSR page + // loads, so this send rejects nearly every time. Left unhandled it reaches + // the Next.js dev overlay. + it("attaches a rejection handler to that send", async () => { + const rejection = Promise.reject({ + code: "NO_REQUEST_LISTENER_FOUND", + message: 'No request listener found for event "x"', + }); + const catchSpy = vi.spyOn(rejection, "catch"); + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + send.mockReturnValue(rejection); + + fireMutation(); + await expect(rejection).rejects.toMatchObject({ + code: "NO_REQUEST_LISTENER_FOUND", + }); + + expect(catchSpy).toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + // postMessageErrors is not mocked here, so the real helper runs. This is + // the case that ties the call site to it: a bare `.catch(() => {})` would + // pass every assertion above but fail this one. + it("warns through the helper when the send fails for another reason", async () => { + // The shape the library's no-ack timeout rejects with: no code. + const rejection = Promise.reject( + "contentstack-adv-post-message: The ACK was not received" + ); + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + send.mockReturnValue(rejection); + + fireMutation(); + await expect(rejection).rejects.toBe( + "contentstack-adv-post-message: The ACK was not received" + ); + + expect(warn).toHaveBeenCalledOnce(); + expect(warn.mock.calls[0][0]).toContain( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + expect(warn.mock.calls[0][1]).toBe( + "contentstack-adv-post-message: The ACK was not received" + ); + }); +}); diff --git a/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts b/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts index 0ccd3941..efba4531 100644 --- a/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts +++ b/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts @@ -39,6 +39,7 @@ import visualBuilderPostMessage from "../../../visualBuilder/utils/visualBuilder import { EventManager } from "@contentstack/advanced-post-message"; import { updateVariantClasses } from "../../../visualBuilder/eventManager/useRecalculateVariantDataCSLPValues"; import * as cslpdata from "../../../cslp/cslpdata"; +import { PublicLogger } from "../../../logger/logger"; const mockVisualBuilderPostMessage = visualBuilderPostMessage as MockedObject; @@ -48,7 +49,9 @@ vi.mock("../../../visualBuilder/utils/visualBuilderPostMessage", () => { return { default: { on: vi.fn(), - send: vi.fn(), + // send always returns a promise; tests that assert on rejection + // handling need the mock to match that contract. + send: vi.fn().mockResolvedValue(undefined), }, }; }); @@ -153,7 +156,8 @@ describe("useVariantFieldsPostMessageEvent", () => { // Reset mocks vi.clearAllMocks(); - + (mockVisualBuilderPostMessage.send as any).mockResolvedValue(undefined); + // Mock isValidCslp to return true for test data (after clearAllMocks) vi.spyOn(cslpdata, "isValidCslp").mockReturnValue(true); }); @@ -613,10 +617,14 @@ describe("useVariantFieldsPostMessageEvent SSR handling", () => { beforeEach(() => { document.querySelectorAll = mockQuerySelectorAll; vi.clearAllMocks(); + // Restate the contract rather than inherit it: send always returns a + // promise, and a test below swaps in a rejecting one. + (mockVisualBuilderPostMessage.send as any).mockResolvedValue(undefined); }); afterEach(() => { document.querySelectorAll = originalQuerySelectorAll; + vi.restoreAllMocks(); }); it("should call addVariantFieldClass directly when isSSR is true and variant is provided", () => { @@ -728,4 +736,72 @@ describe("useVariantFieldsPostMessageEvent SSR handling", () => { ); expect(updateVariantClasses).toHaveBeenCalled(); }); + + // The visual builder registers this listener only while the + // Discussions panel is open, so the send rejects on most page loads. Left + // unhandled it reaches Next.js's unhandledrejection hook and renders a + // "[object Object]" runtime error overlay in dev. + it("attaches a rejection handler to the send", async () => { + useVariantFieldsPostMessageEvent({ isSSR: true }); + const call = mockVisualBuilderPostMessage.on.mock.calls.find( + (call: any[]) => + call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID + ); + const handler = call ? call[1] : null; + + const rejection = Promise.reject({ + code: "NO_REQUEST_LISTENER_FOUND", + message: + 'No request listener found for event "request-discussion-highlights"', + }); + // Deliberately coupled to the `.catch` shape. Asserting "no unhandled + // rejection escapes" would survive a refactor to try/catch, but neither + // a process `unhandledRejection` listener nor the jsdom window event + // fires reliably under vitest here: the same assertion passes with the + // fix removed, so it proves nothing. Rewriting this to catch a + // try/catch refactor means fixing that detection first. + const catchSpy = vi.spyOn(rejection, "catch"); + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + (mockVisualBuilderPostMessage.send as any).mockReturnValue(rejection); + + handler!({ data: { variant: "variant-123" } }); + await expect(rejection).rejects.toMatchObject({ + code: "NO_REQUEST_LISTENER_FOUND", + }); + + expect(catchSpy).toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + // postMessageErrors is not mocked here, so the real helper runs. This is + // the case that ties the call site to it: a bare `.catch(() => {})` would + // pass every assertion above but fail this one. + it("warns through the helper when the send fails for another reason", async () => { + useVariantFieldsPostMessageEvent({ isSSR: true }); + const call = mockVisualBuilderPostMessage.on.mock.calls.find( + (call: any[]) => + call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID + ); + const handler = call ? call[1] : null; + + // The shape the library's no-ack timeout rejects with: no code. + const rejection = Promise.reject( + "contentstack-adv-post-message: The ACK was not received" + ); + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + (mockVisualBuilderPostMessage.send as any).mockReturnValue(rejection); + + handler!({ data: { variant: "variant-123" } }); + await expect(rejection).rejects.toBe( + "contentstack-adv-post-message: The ACK was not received" + ); + + expect(warn).toHaveBeenCalledOnce(); + expect(warn.mock.calls[0][0]).toContain( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + expect(warn.mock.calls[0][1]).toBe( + "contentstack-adv-post-message: The ACK was not received" + ); + }); }); diff --git a/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts b/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts index d44dd5ed..c8810df0 100644 --- a/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts +++ b/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts @@ -6,6 +6,7 @@ import { visualBuilderStyles } from "../visualBuilder.style"; import { isValidCslp } from "../../cslp/cslpdata"; import { setHighlightVariantFields } from "./useVariantsPostMessageEvent"; import visualBuilderPostMessage from "../utils/visualBuilderPostMessage"; +import { ignoreMissingListener } from "../utils/postMessageErrors"; import { VisualBuilderPostMessageEvents } from "../utils/types/postMessage.types"; import { debounce } from "lodash-es"; @@ -14,9 +15,15 @@ const VARIANT_UPDATE_DELAY_MS: Readonly = 8000; // Coalesce a burst of data-cslp mutations into a single request to the // visual editor. const requestDiscussionHighlights = debounce(() => { - visualBuilderPostMessage?.send( - VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS - ); + // The visual builder listens only while the Discussions panel is open, so + // an absent receiver is expected here rather than a failure. + visualBuilderPostMessage + ?.send(VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS) + .catch( + ignoreMissingListener( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ) + ); }, 200); type OnAudienceModeVariantPatchUpdate = { diff --git a/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts b/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts index 33f98443..75d7f19e 100644 --- a/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts +++ b/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts @@ -1,6 +1,7 @@ import { VisualBuilder } from ".."; import { visualBuilderStyles } from "../visualBuilder.style"; import visualBuilderPostMessage from "../utils/visualBuilderPostMessage"; +import { ignoreMissingListener } from "../utils/postMessageErrors"; import { VisualBuilderPostMessageEvents } from "../utils/types/postMessage.types"; import { FieldSchemaMap } from "../utils/fieldSchemaMap"; import { updateVariantClasses } from "./useRecalculateVariantDataCSLPValues"; @@ -167,17 +168,22 @@ export function useVariantFieldsPostMessageEvent({ isSSR }: { isSSR: boolean }): if (selectedVariant) { addVariantFieldClass(selectedVariant); } - // SSR DOM is final; observer never fires, request directly. - visualBuilderPostMessage?.send( - VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS - ); } else { - // CSR: observer in updateVariantClasses requests on settle. + // CSR: observer in updateVariantClasses also requests on settle. updateVariantClasses(); - visualBuilderPostMessage?.send( + } + // Sent in both modes: SSR has no observer to fire it later. The + // visual builder listens only while the Discussions panel is open, + // so an absent receiver is expected here rather than a failure. + visualBuilderPostMessage + ?.send( VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ) + .catch( + ignoreMissingListener( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ) ); - } } ); visualBuilderPostMessage?.on( diff --git a/src/visualBuilder/utils/__test__/postMessageErrors.test.ts b/src/visualBuilder/utils/__test__/postMessageErrors.test.ts new file mode 100644 index 00000000..fc1291a3 --- /dev/null +++ b/src/visualBuilder/utils/__test__/postMessageErrors.test.ts @@ -0,0 +1,54 @@ +import { vi, describe, it, expect, beforeEach } from "vitest"; +import { ignoreMissingListener } from "../postMessageErrors"; +import { VisualBuilderPostMessageEvents } from "../types/postMessage.types"; +import { PublicLogger } from "../../../logger/logger"; + +const EVENT = VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS; + +describe("ignoreMissingListener", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + // The only rejection adv-post-message sends back with a code: the receiver + // acked, looked for a listener and found none. + it("stays silent when the receiver answers that nobody is listening", () => { + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + + ignoreMissingListener(EVENT)({ + code: "NO_REQUEST_LISTENER_FOUND", + message: + 'No request listener found for event "request-discussion-highlights"', + }); + + expect(warn).not.toHaveBeenCalled(); + }); + + // The shapes the library's own timeout paths reject with. Neither carries a + // code, so both must fall through to the warning. + it.each([ + ["an Error, as the closed-window path rejects", new Error("closed")], + [ + "a bare string, as the no-ack timeout rejects", + "contentstack-adv-post-message: The ACK was not received", + ], + ])("warns on %s so a real failure stays visible", (_shape, error) => { + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + + ignoreMissingListener(EVENT)(error); + + expect(warn).toHaveBeenCalledOnce(); + expect(warn.mock.calls[0][0]).toContain(EVENT); + // The rejection itself is the reason this branch exists; without this + // the warn call could drop it and every assertion here would hold. + expect(warn.mock.calls[0][1]).toBe(error); + }); + + it("warns on a coded rejection that is not the missing listener", () => { + const warn = vi.spyOn(PublicLogger, "warn").mockImplementation(() => {}); + + ignoreMissingListener(EVENT)({ code: "SOMETHING_ELSE" }); + + expect(warn).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/visualBuilder/utils/postMessageErrors.ts b/src/visualBuilder/utils/postMessageErrors.ts new file mode 100644 index 00000000..bf141ab5 --- /dev/null +++ b/src/visualBuilder/utils/postMessageErrors.ts @@ -0,0 +1,28 @@ +import { PublicLogger } from "../../logger/logger"; +import type { VisualBuilderPostMessageEvents } from "./types/postMessage.types"; + +// adv-post-message does not export ERROR_CODES from its entry point, so the +// wire value is matched directly. +const NO_REQUEST_LISTENER_FOUND = "NO_REQUEST_LISTENER_FOUND"; + +/** + * Rejection handler for sends whose receiver is only mounted some of the time. + * + * Only the missing-listener reply carries a `code`; the library's other + * rejections are an uncoded `Error` (closed window) or a bare string (no ack), + * so they fall through to the warning. That is deliberate: a receiver that + * never acks is not the same as one that answered "nobody is listening". + */ +export function ignoreMissingListener( + event: VisualBuilderPostMessageEvents +): (error: unknown) => void { + return (error: unknown) => { + if ((error as { code?: string })?.code === NO_REQUEST_LISTENER_FOUND) { + return; + } + PublicLogger.warn( + `Failed to send "${event}" to the visual builder`, + error + ); + }; +}