From 88f46620314f34dc964f8c490179b38d39d26bc7 Mon Sep 17 00:00:00 2001
From: "Mark S. Miller" <erights@gmail.com>
Date: Thu, 6 Apr 2023 14:01:42 -0700
Subject: [PATCH] fix: limit logged args per error

---
 packages/ses/src/error/note-log-args.js       | 38 +++++++++++++------
 packages/ses/test/error/test-lru-cache-map.js | 22 +++++++++++
 packages/ses/test/error/test-note-log-args.js | 24 ++++++++++++
 3 files changed, 73 insertions(+), 11 deletions(-)
 create mode 100644 packages/ses/test/error/test-lru-cache-map.js
 create mode 100644 packages/ses/test/error/test-note-log-args.js

diff --git a/packages/ses/src/error/note-log-args.js b/packages/ses/src/error/note-log-args.js
index ae715407e8..f68cc30a32 100644
--- a/packages/ses/src/error/note-log-args.js
+++ b/packages/ses/src/error/note-log-args.js
@@ -94,16 +94,18 @@ const spliceOut = cell => {
  *
  * TODO: Make parameterized `Key` and `Value` template types
  *
- * @param {number} budget
+ * @param {number} keysBudget
  * @returns {WeakMap<object,any>}
  */
-export const makeLRUCacheMap = budget => {
-  if (!isSafeInteger(budget) || budget < 0) {
-    throw new TypeError('budget must be a safe non-negative integer number');
+export const makeLRUCacheMap = keysBudget => {
+  if (!isSafeInteger(keysBudget) || keysBudget < 0) {
+    throw new TypeError(
+      'keysBudget must be a safe non-negative integer number',
+    );
   }
   /** @type {Map<object, DoublyLinkedCell>} */
   const map = new Map();
-  let size = 0; // `size` must remain <= `budget`
+  let size = 0; // `size` must remain <= `keysBudget`
   // As a sigil, `head` uniquely is not in the `map`.
   const head = makeSelfCell(undefined, undefined);
 
@@ -131,12 +133,12 @@ export const makeLRUCacheMap = budget => {
   freeze(get);
 
   const set = (key, value) => {
-    if (budget >= 1) {
+    if (keysBudget >= 1) {
       let cell = touchCell(key);
       if (cell !== undefined) {
         cell.value = value;
       } else {
-        if (size >= budget) {
+        if (size >= keysBudget) {
           const condemned = head.prev;
           spliceOut(condemned); // Drop least recently used
           map.delete(condemned.key);
@@ -176,12 +178,23 @@ export const makeLRUCacheMap = budget => {
 };
 freeze(makeLRUCacheMap);
 
-const defaultLogArgsBudget = 1000;
+const defaultLoggedErrorsBudget = 1000;
+const defaultArgsPerErrorBudget = 100;
 
 /**
- * @param {number} [budget]
+ * @param {number} [errorsBudget]
+ * @param {number} [argsPerErrorBudget]
  */
-export const makeNoteLogArgsArrayKit = (budget = defaultLogArgsBudget) => {
+export const makeNoteLogArgsArrayKit = (
+  errorsBudget = defaultLoggedErrorsBudget,
+  argsPerErrorBudget = defaultArgsPerErrorBudget,
+) => {
+  if (!isSafeInteger(argsPerErrorBudget) || argsPerErrorBudget < 1) {
+    throw new TypeError(
+      'argsPerErrorBudget must be a safe positive integer number',
+    );
+  }
+
   /**
    * @type {WeakMap<Error, LogArgs[]>}
    *
@@ -192,7 +205,7 @@ export const makeNoteLogArgsArrayKit = (budget = defaultLogArgsBudget) => {
    * An augmented console, like the causal console of `console.js`, could
    * then retrieve the graph of such annotations.
    */
-  const noteLogArgsArrayMap = makeLRUCacheMap(budget);
+  const noteLogArgsArrayMap = makeLRUCacheMap(errorsBudget);
 
   /**
    * @param {Error} error
@@ -201,6 +214,9 @@ export const makeNoteLogArgsArrayKit = (budget = defaultLogArgsBudget) => {
   const addLogArgs = (error, logArgs) => {
     const logArgsArray = noteLogArgsArrayMap.get(error);
     if (logArgsArray !== undefined) {
+      if (logArgsArray.length >= argsPerErrorBudget) {
+        logArgsArray.shift();
+      }
       logArgsArray.push(logArgs);
     } else {
       noteLogArgsArrayMap.set(error, [logArgs]);
diff --git a/packages/ses/test/error/test-lru-cache-map.js b/packages/ses/test/error/test-lru-cache-map.js
new file mode 100644
index 0000000000..d2c84dc6ff
--- /dev/null
+++ b/packages/ses/test/error/test-lru-cache-map.js
@@ -0,0 +1,22 @@
+import test from 'ava';
+
+import { makeLRUCacheMap } from '../../src/error/note-log-args.js';
+
+test('lru cache map basic', t => {
+  const lruMap = makeLRUCacheMap(2);
+  const key1 = {};
+  const key2 = {};
+  const key3 = {};
+  t.is(lruMap.has(key1), false);
+
+  lruMap.set(key1, 'x');
+  lruMap.set(key2, 'y');
+  t.is(lruMap.has(key2), true);
+  t.is(lruMap.has(key1), true);
+  t.is(lruMap.has(key3), false);
+
+  lruMap.set(key3, 'z');
+  t.is(lruMap.has(key1), true);
+  t.is(lruMap.has(key2), false);
+  t.is(lruMap.has(key3), true);
+});
diff --git a/packages/ses/test/error/test-note-log-args.js b/packages/ses/test/error/test-note-log-args.js
new file mode 100644
index 0000000000..8de7c8caf9
--- /dev/null
+++ b/packages/ses/test/error/test-note-log-args.js
@@ -0,0 +1,24 @@
+import test from 'ava';
+
+import { makeNoteLogArgsArrayKit } from '../../src/error/note-log-args.js';
+
+test('note log args array kit basic', t => {
+  const { addLogArgs, takeLogArgsArray } = makeNoteLogArgsArrayKit(3, 2);
+  const e1 = Error('e1');
+  const e2 = Error('e2');
+  const e3 = Error('e3');
+  const e4 = Error('e4');
+
+  addLogArgs(e1, ['a']);
+  addLogArgs(e3, ['b']);
+  addLogArgs(e2, ['c']);
+  addLogArgs(e4, ['d']); // drops e1
+  addLogArgs(e1, ['e']); // new e1 entry, drops e3
+  addLogArgs(e2, ['f']);
+  addLogArgs(e2, ['g']); // drops e2,c
+  addLogArgs(e2, ['h']); // drops e2,f
+  t.deepEqual(takeLogArgsArray(e1), [['e']]);
+  t.deepEqual(takeLogArgsArray(e2), [['g'], ['h']]);
+  t.deepEqual(takeLogArgsArray(e3), undefined);
+  t.deepEqual(takeLogArgsArray(e4), [['d']]);
+});