feat: keep a ring buffer of logs, and allow saving them

Fixes #1301

CHANGELOG.md

@@ -5,6 +5,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 ## Nightly (only)
 
 - feat: simplify pretty print to align with devtools ([vscode#151410](https://github.com/microsoft/vscode/issues/151410))
+- feat: add a new **Debug: Save Diagnostic JS Debug Logs** command ([#1301](https://github.com/microsoft/vscode-js-debug/issues/1301))
 - feat: use custom `toString()` methods to generate object descriptions ([#1284](https://github.com/microsoft/vscode-js-debug/issues/1284))
 - fix: debugged child processes in ext host causing teardown ([#1289](https://github.com/microsoft/vscode-js-debug/issues/1289))
 - fix: errors thrown in process tree lookup not being visible ([vscode#150754](https://github.com/microsoft/vscode/issues/150754))

src/adapter/debugAdapter.ts

@@ -15,7 +15,7 @@ import { AnyLaunchConfiguration } from '../configuration';
 import Dap from '../dap/api';
 import * as errors from '../dap/errors';
 import { ProtocolError } from '../dap/protocolError';
-import { disposeContainer } from '../ioc-extras';
+import { disposeContainer, FS, FsPromises } from '../ioc-extras';
 import { ITarget } from '../targets/targets';
 import { ITelemetryReporter } from '../telemetry/telemetryReporter';
 import { IAsyncStackPolicy } from './asyncStackPolicy';
@@ -115,6 +115,15 @@ export class DebugAdapter implements IDisposable {
     this.dap.on('createDiagnostics', params => this._dumpDiagnostics(params));
     this.dap.on('requestCDPProxy', () => this._requestCDPProxy());
     this.dap.on('setExcludedCallers', params => this._onSetExcludedCallers(params));
+    this.dap.on('saveDiagnosticLogs', ({ toFile }) => this._saveDiagnosticLogs(toFile));
+  }
+
+  private async _saveDiagnosticLogs(toFile: string): Promise<Dap.SaveDiagnosticLogsResult> {
+    const logs = this._services.get<ILogger>(ILogger).getRecentLogs();
+    await this._services
+      .get<FsPromises>(FS)
+      .writeFile(toFile, logs.map(l => JSON.stringify(l)).join('\n'));
+    return {};
   }
 
   public async launchBlocker(): Promise<void> {

src/build/dapCustom.ts

@@ -461,6 +461,20 @@ const dapCustom: JSONSchema4 = {
         required: ['file'],
       },
     ),
+    ...makeRequest(
+      'saveDiagnosticLogs',
+      'Saves recent diagnostic logs for the debug session.',
+      {
+        properties: {
+          toFile: {
+            type: 'string',
+            description: 'File where logs should be saved',
+          },
+        },
+        required: ['toFile'],
+      },
+      {},
+    ),
     ...makeEvent(
       'suggestDiagnosticTool',
       "Shows a prompt to the user suggesting they use the diagnostic tool if breakpoints don't bind.",

src/build/generate-contributions.ts

@@ -1254,6 +1254,11 @@ const commands: ReadonlyArray<{
     title: refString('createDiagnostics.label'),
     category: 'Debug',
   },
+  {
+    command: Commands.GetDiagnosticLogs,
+    title: refString('getDiagnosticLogs.label'),
+    category: 'Debug',
+  },
   {
     command: Commands.StartWithStopOnEntry,
     title: refString('startWithStopOnEntry.label'),
@@ -1323,6 +1328,11 @@ const menus: Menus = {
       title: refString('createDiagnostics.label'),
       when: forAnyDebugType('debugType', 'inDebugMode'),
     },
+    {
+      command: Commands.GetDiagnosticLogs,
+      title: refString('getDiagnosticLogs.label'),
+      when: forAnyDebugType('debugType', 'inDebugMode'),
+    },
     {
       command: Commands.OpenEdgeDevTools,
       title: refString('openEdgeDevTools.label'),

src/build/strings.ts

@@ -307,6 +307,7 @@ A common case to disable certificate verification can be done by passing \`{ "ht
   'profile.stop': 'Stop Performance Profile',
   'debugLink.label': 'Open Link',
   'createDiagnostics.label': 'Diagnose Breakpoint Problems',
+  'getDiagnosticLogs.label': 'Save Diagnostic JS Debug Logs',
   'startWithStopOnEntry.label': 'Start Debugging and Stop on Entry',
   'requestCDPProxy.label': 'Request CDP Proxy for Debug Session',
   'openEdgeDevTools.label': 'Open Browser Devtools',

src/cdp/api.d.ts

@@ -1532,7 +1532,7 @@ export namespace Cdp {
 
     /**
      * Details for issues around "Attribution Reporting API" usage.
-     * Explainer: https://github.com/WICG/conversion-measurement-api
+     * Explainer: https://github.com/WICG/attribution-reporting-api
      */
     export interface AttributionReportingIssueDetails {
       violationType: AttributionReportingIssueType;
@@ -1613,7 +1613,6 @@ export namespace Cdp {
       | 'NotificationInsecureOrigin'
       | 'NotificationPermissionRequestedIframe'
       | 'ObsoleteWebRtcCipherSuite'
-      | 'PaymentRequestBasicCard'
       | 'PictureSourceSrc'
       | 'PrefixedCancelAnimationFrame'
       | 'PrefixedRequestAnimationFrame'
@@ -4959,8 +4958,19 @@ export namespace Cdp {
     ): Promise<Debugger.RemoveBreakpointResult | undefined>;
 
     /**
-     * Restarts particular call frame from the beginning.
-     * @deprecated
+     * Restarts particular call frame from the beginning. The old, deprecated
+     * behavior of `restartFrame` is to stay paused and allow further CDP commands
+     * after a restart was scheduled. This can cause problems with restarting, so
+     * we now continue execution immediatly after it has been scheduled until we
+     * reach the beginning of the restarted frame.
+     *
+     * To stay back-wards compatible, `restartFrame` now expects a `mode`
+     * parameter to be present. If the `mode` parameter is missing, `restartFrame`
+     * errors out.
+     *
+     * The various return values are deprecated and `callFrames` is always empty.
+     * Use the call frames from the `Debugger#paused` events instead, that fires
+     * once V8 pauses at the beginning of the restarted function.
      */
     restartFrame(
       params: Debugger.RestartFrameParams,
@@ -5384,6 +5394,12 @@ export namespace Cdp {
        * Call frame identifier to evaluate on.
        */
       callFrameId: CallFrameId;
+
+      /**
+       * The `mode` parameter must be present and set to 'StepInto', otherwise
+       * `restartFrame` will error out.
+       */
+      mode?: 'StepInto';
     }
 
     /**
@@ -5392,16 +5408,19 @@ export namespace Cdp {
     export interface RestartFrameResult {
       /**
        * New stack trace.
+       * @deprecated
        */
       callFrames: CallFrame[];
 
       /**
        * Async stack trace, if any.
+       * @deprecated
        */
       asyncStackTrace?: Runtime.StackTrace;
 
       /**
        * Async stack trace, if any.
+       * @deprecated
        */
       asyncStackTraceId?: Runtime.StackTraceId;
     }
@@ -11432,12 +11451,21 @@ export namespace Cdp {
        */
       reportProgress?: boolean;
 
+      /**
+       * Deprecated in favor of `exposeInternals`.
+       * @deprecated
+       */
       treatGlobalObjectsAsRoots?: boolean;
 
       /**
        * If true, numerical values are included in the snapshot
        */
       captureNumericValue?: boolean;
+
+      /**
+       * If true, exposes internals of the snapshot.
+       */
+      exposeInternals?: boolean;
     }
 
     /**
@@ -11455,14 +11483,21 @@ export namespace Cdp {
       reportProgress?: boolean;
 
       /**
-       * If true, a raw snapshot without artificial roots will be generated
+       * If true, a raw snapshot without artificial roots will be generated.
+       * Deprecated in favor of `exposeInternals`.
+       * @deprecated
        */
       treatGlobalObjectsAsRoots?: boolean;
 
       /**
        * If true, numerical values are included in the snapshot
        */
       captureNumericValue?: boolean;
+
+      /**
+       * If true, exposes internals of the snapshot.
+       */
+      exposeInternals?: boolean;
     }
 
     /**
@@ -17346,9 +17381,10 @@ export namespace Cdp {
     export type CrossOriginOpenerPolicyValue =
       | 'SameOrigin'
       | 'SameOriginAllowPopups'
+      | 'RestrictProperties'
       | 'UnsafeNone'
       | 'SameOriginPlusCoep'
-      | 'SameOriginAllowPopupsPlusCoep';
+      | 'RestrictPropertiesPlusCoep';
 
     export interface CrossOriginOpenerPolicyStatus {
       value: CrossOriginOpenerPolicyValue;
@@ -20892,6 +20928,12 @@ export namespace Cdp {
        * JavaScript stack trace of when frame was attached, only set if frame initiated from script.
        */
       stack?: Runtime.StackTrace;
+
+      /**
+       * Identifies the bottom-most script which caused the frame to be labelled
+       * as an ad. Only sent if frame is labelled as an ad and id is available.
+       */
+      adScriptId?: AdScriptId;
     }
 
     /**
@@ -21289,6 +21331,23 @@ export namespace Cdp {
       explanations?: AdFrameExplanation[];
     }
 
+    /**
+     * Identifies the bottom-most script which caused the frame to be labelled
+     * as an ad.
+     */
+    export interface AdScriptId {
+      /**
+       * Script Id of the bottom-most script which caused the frame to be labelled
+       * as an ad.
+       */
+      scriptId: Runtime.ScriptId;
+
+      /**
+       * Id of adScriptId's debugger.
+       */
+      debuggerId: Runtime.UniqueDebuggerId;
+    }
+
     /**
      * Indicates whether the frame is a secure context and why it is the case.
      */

src/common/contributionUtils.ts

@@ -32,6 +32,7 @@ export const enum Commands {
   AutoAttachToProcess = 'extension.js-debug.autoAttachToProcess',
   CreateDebuggerTerminal = 'extension.js-debug.createDebuggerTerminal',
   CreateDiagnostics = 'extension.js-debug.createDiagnostics',
+  GetDiagnosticLogs = 'extension.js-debug.getDiagnosticLogs',
   DebugLink = 'extension.js-debug.debugLink',
   DebugNpmScript = 'extension.js-debug.npmScript',
   PickProcess = 'extension.js-debug.pickNodeProcess',
@@ -87,6 +88,7 @@ const commandsObj: { [K in Commands]: null } = {
   [Commands.AutoAttachToProcess]: null,
   [Commands.CreateDebuggerTerminal]: null,
   [Commands.CreateDiagnostics]: null,
+  [Commands.GetDiagnosticLogs]: null,
   [Commands.DebugLink]: null,
   [Commands.DebugNpmScript]: null,
   [Commands.PickProcess]: null,
@@ -184,6 +186,7 @@ export interface ICommandTypes {
     config?: Partial<ITerminalLaunchConfiguration>,
   ): void;
   [Commands.CreateDiagnostics](): void;
+  [Commands.GetDiagnosticLogs](): void;
   [Commands.ToggleSkipping](file: string | number): void;
   [Commands.PrettyPrint](): void;
   [Commands.StartProfile](args?: string | IStartProfileArguments): void;

src/common/logging/index.ts

@@ -96,6 +96,11 @@ export interface ILogger extends IDisposable {
    * Makes an assertion, *logging* if it failed.
    */
   assert<T>(assertion: T | false | undefined | null, message: string): assertion is T;
+
+  /**
+   * Gets recently logged items.
+   */
+  getRecentLogs(): ILogItem<unknown>[];
 }
 
 /**
@@ -119,6 +124,7 @@ export interface ILogSink extends IDisposable {
 export interface ILoggerSetupOptions {
   showWelcome?: boolean;
   sinks: ReadonlyArray<ILogSink>;
+}
 
 /**
  * Fulfills the partial config to a full logging configuration.

src/common/logging/logger.ts

@@ -9,6 +9,25 @@ import { packageName, packageVersion } from '../../configuration';
 import { IDisposable } from '../events';
 import { TestLogSink } from './testLogSink';
 
+/**
+ * Ring buffer used to get logs to diagnose issues.
+ */
+class RingBuffer {
+  private readonly items: ILogItem<unknown>[] = [];
+  private i = 0;
+
+  constructor(private readonly size = 512) {}
+
+  public write(item: ILogItem<unknown>): void {
+    this.items[this.i] = item;
+    this.i = (this.i + 1) % this.size;
+  }
+
+  public read() {
+    return this.items.slice(this.i).concat(this.items.slice(0, this.i));
+  }
+}
+
 /**
  * Implementation of ILogger for the extension.
  */
@@ -20,6 +39,11 @@ export class Logger implements ILogger, IDisposable {
    */
   private logTarget: { queue: ILogItem<unknown>[] } | { sinks: ILogSink[] } = { queue: [] };
 
+  /**
+   * Log buffer for replaying diagnostics.
+   */
+  private readonly logBuffer = new RingBuffer();
+
   /**
    * A no-op logger that never logs anything.
    */
@@ -125,6 +149,8 @@ export class Logger implements ILogger, IDisposable {
    * @inheritdoc
    */
   public log(data: ILogItem<unknown>): void {
+    this.logBuffer.write(data);
+
     if ('queue' in this.logTarget) {
       this.logTarget.queue.push(data);
       return;
@@ -135,6 +161,13 @@ export class Logger implements ILogger, IDisposable {
     }
   }
 
+  /**
+   * @inheritdog
+   */
+  public getRecentLogs() {
+    return this.logBuffer.read();
+  }
+
   /**
    * @inheritdoc
    */

src/common/logging/proxyLogger.ts

@@ -2,7 +2,7 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
-import { ILogger, ILogItem, LogTag, LogLevel } from '.';
+import { ILogger, ILogItem, LogLevel, LogTag } from '.';
 
 export class ProxyLogger implements ILogger {
   private target?: { logger: ILogger } | { queue: ILogItem[] } = { queue: [] };
@@ -91,6 +91,19 @@ export class ProxyLogger implements ILogger {
     }
   }
 
+  /**
+   * @inheritdoc
+   */
+  public getRecentLogs(): ILogItem<unknown>[] {
+    if (!this.target) {
+      return [];
+    } else if ('queue' in this.target) {
+      return this.target.queue;
+    } else {
+      return this.target.logger.getRecentLogs();
+    }
+  }
+
   /**
    * Makes an assertion, *logging* if it failed.
    */