Improve script error dialogs #304

- Include the script's directory path #304.
- Exclude Windows-specific instructions on non-Windows OS.
- Standardize language across dialogs for consistency.

Other supporting changes:

- Add script diagnostics data collection from main process.
- Document script file storage and execution tamper protection in
  SECURITY.md.
- Remove redundant comment in `NodeReadbackFileWriter`.
- Centralize error display for uniformity and simplicity.
- Simpify `WindowVariablesValidator` to omit checks when not on the
  renderer process.
- Improve and centralize Electron environment detection.
- Use more emphatic language (don't worry) in error messages.

tests/unit/presentation/bootstrapping/DependencyProvider.spec.ts

@@ -20,6 +20,7 @@ describe('DependencyProvider', () => {
       useLogger: createTransientTests(),
       useCodeRunner: createTransientTests(),
       useDialog: createTransientTests(),
+      useScriptDiagnosticsCollector: createTransientTests(),
     };
     Object.entries(testCases).forEach(([key, runTests]) => {
       const registeredKey = InjectionKeys[key].key;

tests/unit/presentation/components/Code/CodeButtons/ScriptErrorDialog.spec.ts

@@ -0,0 +1,149 @@
+import { describe, it, expect } from 'vitest';
+import { ScriptDiagnosticsCollector } from '@/application/ScriptDiagnostics/ScriptDiagnosticsCollector';
+import { OperatingSystem } from '@/domain/OperatingSystem';
+import { Dialog } from '@/presentation/common/Dialog';
+import { ScriptErrorDetails, createScriptErrorDialog } from '@/presentation/components/Code/CodeButtons/ScriptErrorDialog';
+import { expectExists } from '@tests/shared/Assertions/ExpectExists';
+import { AllSupportedOperatingSystems } from '@tests/shared/TestCases/SupportedOperatingSystems';
+import { ScriptDiagnosticsCollectorStub } from '@tests/unit/shared/Stubs/ScriptDiagnosticsCollectorStub';
+
+describe('ScriptErrorDialog', () => {
+  describe('handles readback error type', () => {
+    it('handles file readback error', async () => {
+      // arrange
+      const errorDetails = createErrorDetails({ isFileReadbackError: true });
+      const context = new CreateScriptErrorDialogTestSetup()
+        .withDetails(errorDetails);
+      // act
+      const dialog = await context.createScriptErrorDialog();
+      // assert
+      assertValidDialog(dialog);
+    });
+    it('handles generic error', async () => {
+      // arrange
+      const errorDetails = createErrorDetails({ isFileReadbackError: false });
+      const context = new CreateScriptErrorDialogTestSetup()
+        .withDetails(errorDetails);
+      // act
+      const dialog = await context.createScriptErrorDialog();
+      // assert
+      assertValidDialog(dialog);
+    });
+  });
+
+  describe('handles supported operatingSystems', () => {
+    AllSupportedOperatingSystems.forEach((operatingSystem) => {
+      it(`${OperatingSystem[operatingSystem]}`, async () => {
+        // arrange
+        const diagnostics = new ScriptDiagnosticsCollectorStub()
+          .withOperatingSystem(operatingSystem);
+        const context = new CreateScriptErrorDialogTestSetup()
+          .withDiagnostics(diagnostics);
+        // act
+        const dialog = await context.createScriptErrorDialog();
+        // assert
+        assertValidDialog(dialog);
+      });
+    });
+  });
+
+  it('handles undefined diagnostics collector', async () => {
+    const diagnostics = undefined;
+    const context = new CreateScriptErrorDialogTestSetup()
+      .withDiagnostics(diagnostics);
+    // act
+    const dialog = await context.createScriptErrorDialog();
+    // assert
+    assertValidDialog(dialog);
+  });
+
+  it('handles undefined operating system', async () => {
+    // arrange
+    const undefinedOperatingSystem = undefined;
+    const diagnostics = new ScriptDiagnosticsCollectorStub()
+      .withOperatingSystem(undefinedOperatingSystem);
+    const context = new CreateScriptErrorDialogTestSetup()
+      .withDiagnostics(diagnostics);
+    // act
+    const dialog = await context.createScriptErrorDialog();
+    // assert
+    assertValidDialog(dialog);
+  });
+
+  it('handles directory path', async () => {
+    // arrange
+    const undefinedScriptsDirectory = undefined;
+    const diagnostics = new ScriptDiagnosticsCollectorStub()
+      .withScriptDirectoryPath(undefinedScriptsDirectory);
+    const context = new CreateScriptErrorDialogTestSetup()
+      .withDiagnostics(diagnostics);
+    // act
+    const dialog = await context.createScriptErrorDialog();
+    // assert
+    assertValidDialog(dialog);
+  });
+
+  describe('handles all contexts', () => {
+    const possibleContexts: ScriptErrorDetails['errorContext'][] = ['run', 'save'];
+    possibleContexts.forEach((dialogContext) => {
+      it(`${dialogContext} context`, async () => {
+        // arrange
+        const undefinedScriptsDirectory = undefined;
+        const diagnostics = new ScriptDiagnosticsCollectorStub()
+          .withScriptDirectoryPath(undefinedScriptsDirectory);
+        const context = new CreateScriptErrorDialogTestSetup()
+          .withDiagnostics(diagnostics);
+        // act
+        const dialog = await context.createScriptErrorDialog();
+        // assert
+        assertValidDialog(dialog);
+      });
+    });
+  });
+});
+
+function assertValidDialog(dialog: Parameters<Dialog['showError']>): void {
+  expectExists(dialog);
+  const [title, message] = dialog;
+  expectExists(title);
+  expect(title).to.have.length.greaterThan(1);
+  expectExists(message);
+  expect(message).to.have.length.greaterThan(1);
+}
+
+function createErrorDetails(partialDetails?: Partial<ScriptErrorDetails>): ScriptErrorDetails {
+  const defaultDetails: ScriptErrorDetails = {
+    errorContext: 'run',
+    errorType: 'test-error-type',
+    errorMessage: 'test error message',
+    isFileReadbackError: false,
+  };
+  return {
+    ...defaultDetails,
+    ...partialDetails,
+  };
+}
+
+class CreateScriptErrorDialogTestSetup {
+  private details: ScriptErrorDetails = createErrorDetails();
+
+  private diagnostics:
+  ScriptDiagnosticsCollector | undefined = new ScriptDiagnosticsCollectorStub();
+
+  public withDetails(details: ScriptErrorDetails): this {
+    this.details = details;
+    return this;
+  }
+
+  public withDiagnostics(diagnostics: ScriptDiagnosticsCollector | undefined): this {
+    this.diagnostics = diagnostics;
+    return this;
+  }
+
+  public createScriptErrorDialog() {
+    return createScriptErrorDialog(
+      this.details,
+      this.diagnostics,
+    );
+  }
+}

tests/unit/presentation/components/Shared/Hooks/UseScriptDiagnosticsCollector.spec.ts

@@ -0,0 +1,27 @@
+import { describe, it, expect } from 'vitest';
+import { useScriptDiagnosticsCollector } from '@/presentation/components/Shared/Hooks/UseScriptDiagnosticsCollector';
+import { ScriptDiagnosticsCollectorStub } from '@tests/unit/shared/Stubs/ScriptDiagnosticsCollectorStub';
+import { WindowVariables } from '@/infrastructure/WindowVariables/WindowVariables';
+
+describe('useScriptDiagnosticsCollector', () => {
+  it('returns undefined if collector is not present on the window object', () => {
+    // arrange
+    const emptyWindow = {} as WindowVariables;
+    // act
+    const { scriptDiagnosticsCollector } = useScriptDiagnosticsCollector(emptyWindow);
+    // assert
+    expect(scriptDiagnosticsCollector).to.equal(undefined);
+  });
+
+  it('returns the scriptDiagnosticsCollector when it is present on the window object', () => {
+    // arrange
+    const expectedCollector = new ScriptDiagnosticsCollectorStub();
+    const windowWithVariable: Partial<WindowVariables> = {
+      scriptDiagnosticsCollector: expectedCollector,
+    } as Partial<WindowVariables>;
+    // act
+    const { scriptDiagnosticsCollector } = useScriptDiagnosticsCollector(windowWithVariable);
+    // assert
+    expect(scriptDiagnosticsCollector).to.equal(expectedCollector);
+  });
+});

tests/unit/presentation/electron/main/IpcRegistration.spec.ts

@@ -2,12 +2,14 @@ import { describe, it, expect } from 'vitest';
 import { CodeRunnerStub } from '@tests/unit/shared/Stubs/CodeRunnerStub';
 import { ChannelDefinitionKey, IpcChannelDefinitions } from '@/presentation/electron/shared/IpcBridging/IpcChannelDefinitions';
 import {
-  CodeRunnerFactory, DialogFactory, IpcChannelRegistrar, registerAllIpcChannels,
+  CodeRunnerFactory, DialogFactory, IpcChannelRegistrar,
+  ScriptDiagnosticsCollectorFactory, registerAllIpcChannels,
 } from '@/presentation/electron/main/IpcRegistration';
 import { IpcChannel } from '@/presentation/electron/shared/IpcBridging/IpcChannel';
 import { expectExists } from '@tests/shared/Assertions/ExpectExists';
 import { collectExceptionMessage } from '@tests/unit/shared/ExceptionCollector';
 import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub';
+import { ScriptDiagnosticsCollectorStub } from '../../../shared/Stubs/ScriptDiagnosticsCollectorStub';
 
 describe('IpcRegistration', () => {
   describe('registerAllIpcChannels', () => {
@@ -44,6 +46,13 @@ describe('IpcRegistration', () => {
             expectedInstance,
           };
         })(),
+        ScriptDiagnosticsCollector: (() => {
+          const expectedInstance = new ScriptDiagnosticsCollectorStub();
+          return {
+            buildContext: (c) => c.witScriptDiagnosticsCollectorFactory(() => expectedInstance),
+            expectedInstance,
+          };
+        })(),
       };
       Object.entries(testScenarios).forEach(([
         key, { buildContext, expectedInstance },
@@ -79,11 +88,14 @@ describe('IpcRegistration', () => {
 });
 
 class IpcRegistrationTestSetup {
+  private registrar: IpcChannelRegistrar = () => { /* NOOP */ };
+
   private codeRunnerFactory: CodeRunnerFactory = () => new CodeRunnerStub();
 
   private dialogFactory: DialogFactory = () => new DialogStub();
 
-  private registrar: IpcChannelRegistrar = () => { /* NOOP */ };
+  private scriptDiagnosticsCollectorFactory
+  : ScriptDiagnosticsCollectorFactory = () => new ScriptDiagnosticsCollectorStub();
 
   public withRegistrar(registrar: IpcChannelRegistrar): this {
     this.registrar = registrar;
@@ -100,11 +112,19 @@ class IpcRegistrationTestSetup {
     return this;
   }
 
+  public witScriptDiagnosticsCollectorFactory(
+    scriptDiagnosticsCollectorFactory: ScriptDiagnosticsCollectorFactory,
+  ): this {
+    this.scriptDiagnosticsCollectorFactory = scriptDiagnosticsCollectorFactory;
+    return this;
+  }
+
   public run() {
     registerAllIpcChannels(
+      this.registrar,
       this.codeRunnerFactory,
       this.dialogFactory,
-      this.registrar,
+      this.scriptDiagnosticsCollectorFactory,
     );
   }
 }

tests/unit/presentation/electron/preload/ContextBridging/RendererApiProvider.spec.ts

@@ -15,7 +15,9 @@ describe('RendererApiProvider', () => {
       setupContext(context: RendererApiProviderTestContext): RendererApiProviderTestContext;
       readonly expectedValue: unknown;
     }
-    const testScenarios: Record<PropertyKeys<Required<WindowVariables>>, WindowVariableTestCase> = {
+    const testScenarios: Record<
+    PropertyKeys<Required<WindowVariables>>,
+    WindowVariableTestCase> = {
       isRunningAsDesktopApplication: {
         description: 'returns true',
         setupContext: (context) => context,
@@ -32,9 +34,12 @@ describe('RendererApiProvider', () => {
       })(),
       log: expectFacade({
         instance: new LoggerStub(),
-        setupContext: (c, logger) => c.withLogger(logger),
+        setupContext: (c, instance) => c.withLogger(instance),
       }),
       dialog: expectIpcConsumer(IpcChannelDefinitions.Dialog),
+      scriptDiagnosticsCollector: expectIpcConsumer(
+        IpcChannelDefinitions.ScriptDiagnosticsCollector,
+      ),
     };
     Object.entries(testScenarios).forEach((
       [property, { description, setupContext, expectedValue }],
@@ -109,10 +114,10 @@ class RendererApiProviderTestContext {
 
   public provideWindowVariables() {
     return provideWindowVariables(
-      () => this.log,
-      () => this.os,
       this.apiFacadeCreator,
       this.ipcConsumerCreator,
+      () => this.os,
+      () => this.log,
     );
   }
 }

tests/unit/presentation/electron/shared/IpcChannelDefinitions.spec.ts

@@ -16,6 +16,10 @@ describe('IpcChannelDefinitions', () => {
         expectedNamespace: 'dialogs',
         expectedAccessibleMembers: ['saveFile'],
       },
+      ScriptDiagnosticsCollector: {
+        expectedNamespace: 'script-diagnostics-collector',
+        expectedAccessibleMembers: ['collectDiagnosticInformation'],
+      },
     };
     Object.entries(testScenarios).forEach((
       [definitionKey, { expectedNamespace, expectedAccessibleMembers }],