fix(ses,lockdown): make filenames in stacktraces clickable

erights · erights · commit e7158a9d2fb1 · 2025-03-24T16:47:21.000-07:00
diff --git a/packages/lockdown/commit-debug.js b/packages/lockdown/commit-debug.js
@@ -21,13 +21,13 @@ lockdown({
   // NOTE TO REVIEWERS: If you see the following line commented out,
   // this may be a development accident that should be fixed before merging.
   //
-  errorTaming: 'unsafe-debug',
+  errorTaming: 'unsafe',
 
   // The default `{stackFiltering: 'concise'}` setting usually makes for a
   // better debugging experience, by severely reducing the noisy distractions
   // of the normal verbose stack traces. Which is why we comment
-  // out the `'verbose'` setting is commented out below. However, some
-  // tools look for the full filename that it expects in order
+  // out the other settings below. However, some
+  // tools look for the full filename path that it expects in order
   // to fetch the source text for diagnostics,
   //
   // Another reason for not commenting it out: The cause
@@ -36,10 +36,14 @@ lockdown({
   // uncomment out the following line. But please do not commit it in that
   // state.
   //
-  // NOTE TO REVIEWERS: If you see the following line *not* commented out,
-  // this may be a development accident that MUST be fixed before merging.
+  // NOTE TO REVIEWERS: If you see the `stackFiltering` settings *not*
+  // commented out below, this may be a development accident that MUST be
+  // fixed before merging.
   //
-  // stackFiltering: 'verbose',
+  // stackFiltering: 'concise', // Omit frames and shorten paths
+  // stackFiltering: 'omit-frames', // Only omit frames. Do not shorten paths
+  // stackFiltering: 'shorten-paths', // Only shorten paths. Do not omit frames
+  // stackFiltering: 'verbose', // Do not omit frames or shorten paths
 
   // The default `{overrideTaming: 'moderate'}` setting does not hurt the
   // debugging experience much. But it will introduce noise into, for example,
diff --git a/packages/lockdown/pre.js b/packages/lockdown/pre.js
@@ -102,13 +102,12 @@ export const lockdown = defaultOptions => {
       // this may be a development accident that MUST be fixed before merging.
       //
       // errorTaming: 'unsafe',
-      //
-      //
+
       // The default `{stackFiltering: 'concise'}` setting usually makes for a
       // better debugging experience, by severely reducing the noisy distractions
       // of the normal verbose stack traces. Which is why we comment
-      // out the `'verbose'` setting is commented out below. However, some
-      // tools look for the full filename that it expects in order
+      // out the other settings below. However, some
+      // tools look for the full filename path that it expects in order
       // to fetch the source text for diagnostics,
       //
       // Another reason for not commenting it out: The cause
@@ -117,12 +116,15 @@ export const lockdown = defaultOptions => {
       // uncomment out the following line. But please do not commit it in that
       // state.
       //
-      // NOTE TO REVIEWERS: If you see the following line *not* commented out,
-      // this may be a development accident that MUST be fixed before merging.
-      //
-      // stackFiltering: 'verbose',
-      //
+      // NOTE TO REVIEWERS: If you see the `stackFiltering` settings *not*
+      // commented out below, this may be a development accident that MUST be
+      // fixed before merging.
       //
+      // stackFiltering: 'concise', // Omit frames and shorten paths
+      // stackFiltering: 'omit-frames', // Only omit frames. Do not shorten paths
+      // stackFiltering: 'shorten-paths', // Only shorten paths. Do not omit frames
+      // stackFiltering: 'verbose', // Do not omit frames or shorten paths
+
       // The default `{overrideTaming: 'moderate'}` setting does not hurt the
       // debugging experience much. But it will introduce noise into, for example,
       // the vscode debugger's object inspector. During debug and test, if you can
@@ -134,8 +136,7 @@ export const lockdown = defaultOptions => {
       // this may be a development accident that MUST be fixed before merging.
       //
       // overrideTaming: 'min',
-      //
-      //
+
       // The default `{consoleTaming: 'safe'}` setting usually makes for a
       // better debugging experience, by wrapping the original `console` with
       // the SES replacement `console` that provides more information about
diff --git a/packages/ses-ava/test/raw-ava-reject.test.js b/packages/ses-ava/test/raw-ava-reject.test.js
@@ -15,8 +15,10 @@ lockdown({
   // the current relevant environment variable setting. To get results
   // independent of that, always uncomment one setting for each switch.
   //
-  stackFiltering: 'concise', // Default. Hide infrastructure, shorten paths
-  // stackFiltering: 'verbose', // Include `assert` infrastructure
+  stackFiltering: 'concise', // Default. Omit likely uninteresting frames. Shorten paths
+  // stackFiltering: 'omit-frames', // Only omit infrastructure frames
+  // stackFiltering: 'shorten-paths', // Only shorten paths
+  // stackFiltering: 'verbose', // Original frames with original paths
   consoleTaming: 'safe', // Default. Console with access to redacted info
   // consoleTaming: 'unsafe', // Host console lacks access to redacted info
   // errorTaming: 'safe', // Default. Hide redacted info on error
diff --git a/packages/ses-ava/test/raw-ava-throw.test.js b/packages/ses-ava/test/raw-ava-throw.test.js
@@ -15,8 +15,10 @@ lockdown({
   // the current relevant environment variable setting. To get results
   // independent of that, always uncomment one setting for each switch.
   //
-  stackFiltering: 'concise', // Default. Hide infrastructure, shorten paths
-  // stackFiltering: 'verbose', // Include `assert` infrastructure
+  stackFiltering: 'concise', // Default. Omit likely uninteresting frames. Shorten paths
+  // stackFiltering: 'omit-frames', // Only omit infrastructure frames
+  // stackFiltering: 'shorten-paths', // Only shorten paths
+  // stackFiltering: 'verbose', // Original frames with original paths
   consoleTaming: 'safe', // Default. Console with access to redacted info
   // consoleTaming: 'unsafe', // Host console lacks access to redacted info
   // errorTaming: 'safe', // Default. Hide redacted info on error
diff --git a/packages/ses-ava/test/ses-ava-reject.test.js b/packages/ses-ava/test/ses-ava-reject.test.js
@@ -16,8 +16,10 @@ lockdown({
   // the current relevant environment variable setting. To get results
   // independent of that, always uncomment one setting for each switch.
   //
-  stackFiltering: 'concise', // Default. Hide infrastructure, shorten paths
-  // stackFiltering: 'verbose', // Include `assert` infrastructure
+  stackFiltering: 'concise', // Default. Omit likely uninteresting frames. Shorten paths
+  // stackFiltering: 'omit-frames', // Only omit infrastructure frames
+  // stackFiltering: 'shorten-paths', // Only shorten paths
+  // stackFiltering: 'verbose', // Original frames with original paths
   consoleTaming: 'safe', // Default. Console with access to redacted info
   // consoleTaming: 'unsafe', // Host console lacks access to redacted info
   // errorTaming: 'safe', // Default. Hide redacted info on error
diff --git a/packages/ses-ava/test/ses-ava-throw.test.js b/packages/ses-ava/test/ses-ava-throw.test.js
@@ -16,8 +16,10 @@ lockdown({
   // the current relevant environment variable setting. To get results
   // independent of that, always uncomment one setting for each switch.
   //
-  stackFiltering: 'concise', // Default. Hide infrastructure, shorten paths
-  // stackFiltering: 'verbose', // Include `assert` infrastructure
+  stackFiltering: 'concise', // Default. Omit likely uninteresting frames. Shorten paths
+  // stackFiltering: 'omit-frames', // Only omit infrastructure frames
+  // stackFiltering: 'shorten-paths', // Only shorten paths
+  // stackFiltering: 'verbose', // Original frames with original paths
   consoleTaming: 'safe', // Default. Console with access to redacted info
   // consoleTaming: 'unsafe', // Host console lacks access to redacted info
   // errorTaming: 'safe', // Default. Hide redacted info on error
diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md
@@ -1,9 +1,18 @@
 User-visible changes in `ses`:
 
-# v1.12.0 (2025-03-11)
+# Next release
+
+- Two new `stackFiltering:` options are added
+  - `'omit-frames'` -- Only omit likely uninteresting frames. Keep original paths.
+  - `'shorten-paths'` -- Only shorten paths to text likely clickable in an IDE
 
-- The `evalTaming:` option values are renamed:
+  This fills out the matrix of what should have been orthogonal options.
+  The existing `'concise'` setting both omits likely uninteresting frames and
+  shortens their paths. The existing `'verbose'` setting does neither.
+
+# v1.12.0 (2025-03-11)
 
+- The `evalTaming:` option values are renamed
   - from `'safeEval'`, `'unsafeEval'`, and `'noEval'`
   - to `'safe-eval'`, `'unsafe-eval'`, and `'no-eval'`
 
diff --git a/packages/ses/docs/lockdown.md b/packages/ses/docs/lockdown.md
@@ -30,7 +30,7 @@ Each option is explained in its own section below.
 | `reporting`                      | `'platform'`     | `'console'` `'none'`                   | where to report warnings ([details](#reporting-options))
 | `unhandledRejectionTrapping`     | `'report'`       | `'none'`                               | handling of finalized unhandled rejections ([details](#unhandledrejectiontrapping-options)) |
 | `evalTaming`                     | `'safe-eval'`    | `'unsafe-eval'` `'no-eval'`            | `eval` and `Function` of the start compartment ([details](#evaltaming-options)) |
-| `stackFiltering`                 | `'concise'`      | `'verbose'`                            | deep stacks signal/noise   ([details](#stackfiltering-options)) |
+| `stackFiltering`                 | `'concise'`      | `'omit-frames'` `'shorten-paths'` `'verbose'`  | deep stacks signal/noise   ([details](#stackfiltering-options)) |
 | `overrideTaming`                 | `'moderate'`     | `'min'` or `'severe'`                  | override mistake antidote  ([details](#overridetaming-options)) |
 | `overrideDebug`                  | `[]`             | array of property names                | detect override mistake    ([details](#overridedebug-options)) |
 | `domainTaming`                   | `'safe'`         | `'unsafe'`                             | Node.js `domain` module    ([details](#domaintaming-options)) |
@@ -633,7 +633,11 @@ bugs.
 ```js
 lockdown(); // stackFiltering defaults to 'concise'
 // or
-lockdown({ stackFiltering: 'concise' }); // Preserve important deep stack info
+lockdown({ stackFiltering: 'concise' }); // Preserve important deep stack info. Omit likely uninteresting frames. Shorten paths to likely clickable strings in an IDE
+// vs
+lockdown({ stackFiltering: 'omit-frames' }); // Only omit likely uninteresting frames. Preserve original paths
+// vs
+lockdown({ stackFiltering: 'shorten-paths' }); // Preserve original frames. shorten their paths to likely clickable strings in an IDE.
 // vs
 lockdown({ stackFiltering: 'verbose' }); // Console shows full deep stacks
 ```
@@ -643,6 +647,8 @@ If `lockdown` does not receive a `stackFiltering` option, it will respect
 
 ```console
 LOCKDOWN_STACK_FILTERING=concise
+LOCKDOWN_STACK_FILTERING=omit-frames
+LOCKDOWN_STACK_FILTERING=shorten-paths
 LOCKDOWN_STACK_FILTERING=verbose
 ```
 
@@ -659,7 +665,7 @@ that information is no longer an extraneous distraction. Sometimes the noise
 you filter out actually contains the signal you're looking for. The
 `'verbose'` setting shows, on the console, the full raw stack information
 for each level of the deep stacks.
-Either setting of `stackFiltering` setting is safe. Stack information will
+Any setting of `stackFiltering` is safe. Stack information will
 or will not be available from error objects according to the `errorTaming`
 option and the platform error behavior.
 
diff --git a/packages/ses/docs/reference.md b/packages/ses/docs/reference.md
@@ -178,8 +178,10 @@ below.
   </tr>
   <tr>
     <td><code>stackFiltering</code></td>
-    <td><code>'concise'</code> (default) or <code>'verbose'</code></td>
-    <td><code>'concise'</code> preserves important deep stack info,<br>
+    <td><code>'concise'</code> (default) or <code>'omit-frames'</code>  or <code>'shorten-paths'</code> or <code>'verbose'</code></td>
+    <td><code>'concise'</code> preserves important deep stack info, omitting likely unimportant frames and shortening paths<br>
+        <code>'omit-frames'</code> omits likely unimportant frames<br>
+        <code>'shorten-paths'</code> shortens paths to text likely clickable in an IDE<br>
         <code>'verbose'</code> console shows full deep stacks</td>
   </tr>
   <tr>
@@ -314,7 +316,11 @@ option and the platform error behavior.
 ```js
 lockdown(); // stackFiltering defaults to 'concise'
 // or
-lockdown({ stackFiltering: 'concise' }); // Preserve important deep stack info
+lockdown({ stackFiltering: 'concise' }); // Preserve important deep stack info, omitting likely unimportant frames and shortening paths
+// vs
+lockdown({ stackFiltering: 'omit-frames' }); // Omit likely uninteresting frames
+// vs
+lockdown({ stackFiltering: 'shorten-paths' }); // Shorten paths to text likely clickable in an IDE
 // vs
 lockdown({ stackFiltering: 'verbose' }); // Console shows full deep stacks
 ```
diff --git a/packages/ses/src/error/tame-v8-error-constructor.js b/packages/ses/src/error/tame-v8-error-constructor.js
@@ -120,7 +120,19 @@ export const filterFileName = fileName => {
 //
 // See thread starting at
 // https://github.com/Agoric/agoric-sdk/issues/2326#issuecomment-773020389
-const CALLSITE_ELLIPSES_PATTERN = /^((?:.*[( ])?)[:/\w_-]*\/\.\.\.\/(.+)$/;
+const CALLSITE_ELLIPSIS_PATTERN1 = /^((?:.*[( ])?)[:/\w_-]*\/\.\.\.\/(.+)$/;
+
+// The ad-hoc rule of the current pattern is that any likely-file-path or
+// likely url-path prefix consisting of `.../` should get dropped.
+// Anything to the left of the likely path text is kept.
+// Everything to the right of `.../` is kept. Thus
+// `'Object.bar (.../eventual-send/test/test-deep-send.js:13:21)'`
+// simplifies to
+// `'Object.bar (eventual-send/test/test-deep-send.js:13:21)'`.
+//
+// See thread starting at
+// https://github.com/Agoric/agoric-sdk/issues/2326#issuecomment-773020389
+const CALLSITE_ELLIPSIS_PATTERN2 = /^((?:.*[( ])?)\.\.\.\/(.+)$/;
 
 // The ad-hoc rule of the current pattern is that any likely-file-path or
 // likely url-path prefix, ending in a `/` and prior to `package/` should get
@@ -134,21 +146,38 @@ const CALLSITE_ELLIPSES_PATTERN = /^((?:.*[( ])?)[:/\w_-]*\/\.\.\.\/(.+)$/;
 // lerna.
 const CALLSITE_PACKAGES_PATTERN = /^((?:.*[( ])?)[:/\w_-]*\/(packages\/.+)$/;
 
+// The ad-hoc rule of the current pattern is that any likely-file-path or
+// likely url-path prefix of the form `file://` but not `file:///` gets
+// dropped.
+// Anything to the left of the likely path prefix text is kept. Everything to
+// the right of `file://` is kept. Thus
+// `'Object.bar (file:///Users/markmiller/src/ongithub/agoric/agoric-sdk/packages/eventual-send/test/test-deep-send.js:13:21)'` is unchanged but
+// `'Object.bar (file://test/test-deep-send.js:13:21)'`
+
+// simplifies to
+// `'Object.bar (test/test-deep-send.js:13:21)'`.
+const CALLSITE_FILE_2SLASH_PATTERN = /^((?:.*[( ])?)file:\/\/([^/].*)$/;
+
 // The use of these callSite patterns below assumes that any match will bind
 // capture groups containing the parts of the original string we want
 // to keep. The parts outside those capture groups will be dropped from concise
 // stacks.
 // TODO Enable users to configure CALLSITE_PATTERNS via `lockdown` options.
 const CALLSITE_PATTERNS = [
-  CALLSITE_ELLIPSES_PATTERN,
+  CALLSITE_ELLIPSIS_PATTERN1,
+  CALLSITE_ELLIPSIS_PATTERN2,
   CALLSITE_PACKAGES_PATTERN,
+  CALLSITE_FILE_2SLASH_PATTERN,
 ];
 
 // For a stack frame that should be included in a concise stack trace, if
 // `callSiteString` is the original stringified stack frame, return the
 // possibly-shorter stringified stack frame that should be shown instead.
 // Exported only so it can be unit tested.
 // TODO Move so that it applies not just to v8.
+/**
+ * @param {string} callSiteString
+ */
 export const shortenCallSiteString = callSiteString => {
   for (const filter of CALLSITE_PATTERNS) {
     const match = regexpExec(filter, callSiteString);
@@ -175,18 +204,24 @@ export const tameV8ErrorConstructor = (
 
   const originalCaptureStackTrace = OriginalError.captureStackTrace;
 
+  const omitFrames =
+    stackFiltering === 'concise' || stackFiltering === 'omit-frames';
+
+  const shortenPaths =
+    stackFiltering === 'concise' || stackFiltering === 'shorten-paths';
+
   // const callSiteFilter = _callSite => true;
   const callSiteFilter = callSite => {
-    if (stackFiltering === 'verbose') {
-      return true;
+    if (omitFrames) {
+      // eslint-disable-next-line @endo/no-polymorphic-call
+      return filterFileName(callSite.getFileName());
     }
-    // eslint-disable-next-line @endo/no-polymorphic-call
-    return filterFileName(callSite.getFileName());
+    return true;
   };
 
   const callSiteStringifier = callSite => {
     let callSiteString = `${callSite}`;
-    if (stackFiltering === 'concise') {
+    if (shortenPaths) {
       callSiteString = shortenCallSiteString(callSiteString);
     }
     return `\n  at ${callSiteString}`;
diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js
@@ -189,7 +189,11 @@ export const repairIntrinsics = (options = {}) => {
     overrideTaming = /** @type {'moderate' | 'min' | 'severe'} */ (
       getenv('LOCKDOWN_OVERRIDE_TAMING', 'moderate', ['min', 'severe'])
     ),
-    stackFiltering = getenv('LOCKDOWN_STACK_FILTERING', 'concise', ['verbose']),
+    stackFiltering = getenv('LOCKDOWN_STACK_FILTERING', 'concise', [
+      'omit-frames',
+      'shorten-paths',
+      'verbose',
+    ]),
     domainTaming = getenv('LOCKDOWN_DOMAIN_TAMING', 'safe', ['unsafe']),
     evalTaming = getenv('LOCKDOWN_EVAL_TAMING', 'safe-eval', [
       'unsafe-eval',
diff --git a/packages/ses/types.d.ts b/packages/ses/types.d.ts
@@ -43,7 +43,7 @@ export interface RepairOptions {
     | 'safeEval'
     | 'unsafeEval'
     | 'noEval';
-  stackFiltering?: 'concise' | 'verbose';
+  stackFiltering?: 'concise' | 'omit-frames' | 'shorten-paths' | 'verbose';
   overrideTaming?: 'moderate' | 'min' | 'severe';
   overrideDebug?: Array<string>;
   domainTaming?: 'safe' | 'unsafe';
