Merge branch 'main' into split-playwright-runs

package.json

@@ -38,8 +38,8 @@
     "@apidevtools/json-schema-ref-parser": "^10.1.0",
     "@atlaskit/tree": "^8.8.9",
     "@cfworker/json-schema": "^1.12.8",
-    "@datadog/browser-logs": "^5.19.0",
-    "@datadog/browser-rum": "^5.19.0",
+    "@datadog/browser-logs": "^5.20.0",
+    "@datadog/browser-rum": "^5.20.0",
     "@floating-ui/dom": "^1.6.5",
     "@fortawesome/fontawesome-svg-core": "1.2.36",
     "@fortawesome/free-brands-svg-icons": "^5.15.4",
@@ -223,7 +223,7 @@
     "@types/mark.js": "^8.11.12",
     "@types/marked": "^6.0.0",
     "@types/mustache": "^4.2.5",
-    "@types/node": "^20.13.0",
+    "@types/node": "^20.14.2",
     "@types/nunjucks": "^3.2.6",
     "@types/object-hash": "^2.1.1",
     "@types/papaparse": "^5.3.14",
@@ -247,7 +247,7 @@
     "@types/webpack": "^5.28.5",
     "@types/webpack-env": "^1.18.5",
     "@types/whatwg-mimetype": "^3.0.2",
-    "@typescript-eslint/rule-tester": "^7.10.0",
+    "@typescript-eslint/rule-tester": "^7.12.0",
     "axios-mock-adapter": "^1.22.0",
     "blob-polyfill": "^7.0.20220408",
     "compass-mixins": "^0.12.10",
@@ -269,7 +269,7 @@
     "jest-extended": "^4.0.2",
     "jest-location-mock": "^2.0.0",
     "jest-webextension-mock": "^3.9.0",
-    "jsdom": "^24.0.0",
+    "jsdom": "^24.1.0",
     "jsdom-testing-mocks": "^1.13.0",
     "knip": "^5.16.0",
     "mini-css-extract-plugin": "^2.6.1",
@@ -289,11 +289,11 @@
     "style-loader": "^4.0.0",
     "terser-webpack-plugin": "^5.3.10",
     "ts-loader": "^9.5.1",
-    "type-fest": "^4.18.3",
+    "type-fest": "^4.20.0",
     "typescript": "^5.4.5",
     "typescript-plugin-css-modules": "^5.1.0",
     "webpack": "^5.91.0",
-    "webpack-build-notifier": "^2.3.0",
+    "webpack-build-notifier": "^3.1.0",
     "webpack-bundle-analyzer": "^4.10.2",
     "webpack-cli": "^5.1.4",
     "webpack-dev-server": "^5.0.4",

src/pageEditor/tabs/trigger/TriggerConfiguration.tsx

@@ -245,16 +245,16 @@ const TriggerConfiguration: React.FC<{
         title="Telemetry Mode"
         description={
           <p>
-            Events/errors to report telemetry. Select &ldquo;Report All Events
-            and Errors&rdquo; to report all runs and errors. Select
-            &ldquo;Report First Event and Error&rdquo; to only report the first
-            run and first error.
+            Configures which events (also known as runs) and errors to report in
+            mod telemetry.
           </p>
         }
         {...makeLockableFieldProps("Telemetry Mode", isLocked)}
       >
         <option value="all">Report All Events and Errors</option>
         <option value="once">Report First Event and Error</option>
+        <option value="error-once">Report First Error</option>
+        <option value="never">Never Report Events or Errors</option>
       </ConnectedFieldTemplate>
 
       <MatchRulesSection isLocked={isLocked} />

src/runtime/pipelineTests/pipelineTestHelpers.ts

@@ -190,6 +190,39 @@ export class ThrowBrick extends BrickABC {
   }
 }
 
+export class ThrowTwiceBrick extends BrickABC {
+  static BRICK_ID = validateRegistryId("test/throw-twice");
+  private timesThrown: number;
+  private get maxThrows() {
+    return 2;
+  }
+
+  constructor() {
+    super(ThrowTwiceBrick.BRICK_ID, "Throw Twice Brick");
+    this.timesThrown = 0;
+  }
+
+  inputSchema = propertiesToSchema(
+    {
+      message: {
+        type: "string",
+      },
+    },
+    [],
+  );
+
+  async run({
+    message = "Default Business Error",
+  }: BrickArgs<{ message?: string }>) {
+    if (this.maxThrows && this.timesThrown >= this.maxThrows) {
+      return { prop: "no error!" };
+    }
+
+    this.timesThrown += 1;
+    throw new BusinessError(message);
+  }
+}
+
 class ArrayBrick extends BrickABC {
   constructor() {
     super("test/array", "Array Brick");

src/starterBricks/triggerExtension.test.ts

@@ -43,10 +43,12 @@ import { uuidSequence } from "@/testUtils/factories/stringFactories";
 import {
   throwBrick,
   ThrowBrick,
+  ThrowTwiceBrick,
 } from "@/runtime/pipelineTests/pipelineTestHelpers";
 import { showNotification } from "@/utils/notify";
 import { notifyContextInvalidated } from "@/errors/contextInvalidated";
 import reportError from "@/telemetry/reportError";
+import reportEvent from "@/telemetry/reportEvent";
 import { screen } from "@testing-library/react";
 import type { Trigger } from "@/starterBricks/triggerExtensionTypes";
 import { getPlatform } from "@/platform/platformContext";
@@ -72,6 +74,7 @@ jest.mock("@/errors/contextInvalidated", () => {
 jest.mock("@/utils/notify");
 
 const reportErrorMock = jest.mocked(reportError);
+const reportEventMock = jest.mocked(reportEvent);
 const showNotificationMock = jest.mocked(showNotification);
 const notifyContextInvalidatedMock = jest.mocked(notifyContextInvalidated);
 
@@ -122,10 +125,16 @@ beforeEach(() => {
   window.document.body.innerHTML = "";
   document.body.innerHTML = "";
   reportErrorMock.mockReset();
+  reportEventMock.mockReset();
   showNotificationMock.mockReset();
   notifyContextInvalidatedMock.mockReset();
   blockRegistry.clear();
-  blockRegistry.register([rootReader, new InvalidContextReader(), throwBrick]);
+  blockRegistry.register([
+    rootReader,
+    new InvalidContextReader(),
+    throwBrick,
+    new ThrowTwiceBrick(),
+  ]);
   rootReader.readCount = 0;
   rootReader.ref = null;
 });
@@ -543,7 +552,7 @@ describe("triggerExtension", () => {
     expect(notifyContextInvalidatedMock).toHaveBeenCalledTimes(2);
   });
 
-  it("reports only the first brick error for reportMode: once", async () => {
+  it("reports only the first brick error or event for reportMode: once", async () => {
     const extensionPoint = fromJS(
       getPlatform(),
       extensionPointFactory({
@@ -553,6 +562,86 @@ describe("triggerExtension", () => {
       })({}),
     );
 
+    const modComponent = extensionFactory({
+      extensionPointId: extensionPoint.id,
+      config: {
+        action: { id: ThrowTwiceBrick.BRICK_ID, config: {} },
+      },
+    });
+
+    extensionPoint.registerModComponent(modComponent);
+
+    await extensionPoint.install();
+
+    // Run 4x
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will throw an error
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will throw another error
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will run successfully
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will also run successfully
+
+    // Does not report successful event only once
+    expect(reportEventMock).toHaveBeenCalledExactlyOnceWith("TriggerRun", {
+      extensionId: modComponent.id,
+    });
+
+    // Reports an error once
+    expect(reportErrorMock).toHaveBeenCalledTimes(1);
+    expect(showNotificationMock).toHaveBeenCalledExactlyOnceWith({
+      type: "error",
+      message: "An error occurred running a trigger",
+      reportError: false,
+      error: expect.toBeObject(),
+    });
+  });
+
+  it("reports only the first brick error for reportMode: error-once", async () => {
+    const extensionPoint = fromJS(
+      getPlatform(),
+      extensionPointFactory({
+        trigger: "load",
+        reportMode: "error-once",
+        showErrors: true,
+      })({}),
+    );
+
+    const modComponent = extensionFactory({
+      extensionPointId: extensionPoint.id,
+      config: {
+        action: { id: ThrowTwiceBrick.BRICK_ID, config: {} },
+      },
+    });
+
+    extensionPoint.registerModComponent(modComponent);
+
+    // Run 4x
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will throw an error
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will throw another error
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will run successfully
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL }); // Will also run successfully
+
+    // Reports a successful event only once
+    expect(reportEventMock).not.toHaveBeenCalled();
+
+    // Reports an error once
+    expect(reportErrorMock).toHaveBeenCalledTimes(1);
+    expect(showNotificationMock).toHaveBeenCalledExactlyOnceWith({
+      type: "error",
+      message: "An error occurred running a trigger",
+      reportError: false,
+      error: expect.toBeObject(),
+    });
+  });
+
+  it("never reports brick errors for reportMode: never", async () => {
+    const extensionPoint = fromJS(
+      getPlatform(),
+      extensionPointFactory({
+        trigger: "load",
+        reportMode: "never",
+        showErrors: true,
+      })({}),
+    );
+
     extensionPoint.registerModComponent(
       extensionFactory({
         extensionPointId: extensionPoint.id,
@@ -564,18 +653,10 @@ describe("triggerExtension", () => {
 
     await extensionPoint.install();
 
-    // Run 2x
     await extensionPoint.runModComponents({ reason: RunReason.MANUAL });
-    await extensionPoint.runModComponents({ reason: RunReason.MANUAL });
-
-    expect(reportErrorMock).toHaveBeenCalledTimes(1);
 
-    expect(showNotificationMock).toHaveBeenCalledExactlyOnceWith({
-      type: "error",
-      message: "An error occurred running a trigger",
-      reportError: false,
-      error: expect.toBeObject(),
-    });
+    expect(reportErrorMock).not.toHaveBeenCalled();
+    expect(showNotificationMock).not.toHaveBeenCalled();
   });
 
   it("reports all brick errors for reportMode: all, showErrors: true", async () => {
@@ -636,6 +717,37 @@ describe("triggerExtension", () => {
     expect(reportErrorMock).toHaveBeenCalledTimes(2);
     expect(showNotificationMock).toHaveBeenCalledTimes(0);
   });
+
+  it("reports error notifications, but does not display them for reportMode: default, showErrors: default", async () => {
+    const extensionPoint = fromJS(
+      getPlatform(),
+      extensionPointFactory({
+        trigger: "load",
+        // Testing the default of error-once, for backward compatability
+        // reportMode: "error-once",
+        // Testing the default of false, for backward compatability
+        // showErrors: false,
+      })({}),
+    );
+
+    extensionPoint.registerModComponent(
+      extensionFactory({
+        extensionPointId: extensionPoint.id,
+        config: {
+          action: { id: ThrowBrick.BRICK_ID, config: {} },
+        },
+      }),
+    );
+
+    await extensionPoint.install();
+
+    // Run 2x
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL });
+    await extensionPoint.runModComponents({ reason: RunReason.MANUAL });
+
+    expect(reportErrorMock).toHaveBeenCalledTimes(1);
+    expect(showNotificationMock).not.toHaveBeenCalled();
+  });
 });
 
 describe("defaults", () => {

src/starterBricks/triggerExtension.ts

@@ -100,7 +100,9 @@ export type TriggerConfig = {
 export function getDefaultReportModeForTrigger(
   trigger: Nullishable<Trigger>,
 ): ReportMode {
-  return trigger && USER_ACTION_TRIGGERS.includes(trigger) ? "all" : "once";
+  return trigger && USER_ACTION_TRIGGERS.includes(trigger)
+    ? "all"
+    : "error-once";
 }
 
 /**
@@ -215,15 +217,30 @@ export abstract class TriggerStarterBrickABC extends StarterBrickABC<TriggerConf
   readonly capabilities: PlatformCapability[] = ["dom", "state"];
 
   /**
-   * Returns true if an event should be reported, given whether it has already been reported.
-   * @param alreadyReported true if the event has already been reported
+   * Returns true if an event or error should be reported, given whether it has already been reported.
+   * @param alreadyReported true if the event or error has already been reported
+   * @param isError true if reporting an error
    */
-  private shouldReport(alreadyReported: boolean): boolean {
+  private shouldReport({
+    alreadyReported,
+    isError,
+  }: {
+    alreadyReported: boolean;
+    isError: boolean;
+  }): boolean {
     switch (this.reportMode) {
       case "once": {
         return !alreadyReported;
       }
 
+      case "error-once": {
+        return isError && !alreadyReported;
+      }
+
+      case "never": {
+        return false;
+      }
+
       case "all": {
         return true;
       }
@@ -257,7 +274,7 @@ export abstract class TriggerStarterBrickABC extends StarterBrickABC<TriggerConf
     if (extensionId) {
       const alreadyReported = this.reportedErrors.has(extensionId);
       this.reportedErrors.add(extensionId);
-      return this.shouldReport(alreadyReported);
+      return this.shouldReport({ alreadyReported, isError: true });
     }
 
     return true;
@@ -269,7 +286,7 @@ export abstract class TriggerStarterBrickABC extends StarterBrickABC<TriggerConf
   private shouldReportEvent(componentId: UUID): boolean {
     const alreadyReported = this.reportedEvents.has(componentId);
     this.reportedEvents.add(componentId);
-    return this.shouldReport(alreadyReported);
+    return this.shouldReport({ alreadyReported, isError: false });
   }
 
   async install(): Promise<boolean> {