From ea92d83744fc77ada8d2946bd7b162f05382d9bd Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@meteor.com>
Date: Tue, 27 Sep 2016 10:13:19 -0400
Subject: [PATCH] Don't cache exceptions in optimistic functions.

I originally added the exception-caching functionality in order to avoid
calling files.stat repeatedly for files known to be missing, but now that
we're using statOrNull, knowledge of missing files (as indicated by
statOrNull returning null) is being properly cached.

The reason it's dangerous to cache exceptions is that (for example) when
an ENOENT exception indicates the file is missing, there will be no more
change events for that file, effectively making the exception permanent,
even if the file comes to exist at a later time.
---
 tools/fs/optimistic.js | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/tools/fs/optimistic.js b/tools/fs/optimistic.js
index a96886cc09..31e14588f1 100644
--- a/tools/fs/optimistic.js
+++ b/tools/fs/optimistic.js
@@ -12,6 +12,8 @@ import {
   readdir,
 } from "./files.js";
 
+const CACHE_MISS_SENTINEL = Object.create(null);
+
 function makeOptimistic(name, fn) {
   assert.strictEqual(typeof fn, "function");
 
@@ -46,23 +48,17 @@ function makeOptimistic(name, fn) {
       }
 
       entry.results = Object.create(null);
-
-      // Equivalent to a cache miss, because there is no "value" key.
-      entry.results[key] = { error: false };
+      entry.results[key] = CACHE_MISS_SENTINEL;
 
       cache.set(path, entry);
     }
 
-    const result = entry.results[key];
-
-    if ("value" in result) {
-      if (result.error) {
-        throw result.value;
-      }
-      return result.value;
+    let result = entry.results[key];
+    if (result === CACHE_MISS_SENTINEL) {
+      result = entry.results[key] = fn(...args);
     }
 
-    return tryApply(result, fn, args);
+    return result;
   });
 }
 
@@ -79,10 +75,8 @@ function maybeStartWatcher(path, key) {
 
   try {
     entry.watcher = watch(path, (...args) => {
-      const result = entry.results[key];
       // Trigger a cache miss the next time anyone asks.
-      delete result.value;
-      result.error = false;
+      entry.results[key] = CACHE_MISS_SENTINEL;
     });
 
   } catch (e) {
@@ -93,18 +87,6 @@ function maybeStartWatcher(path, key) {
   return entry;
 }
 
-function tryApply(result, fn, args) {
-  try {
-    result.value = fn(...args)
-    result.error = false;
-  } catch (e) {
-    result.error = true;
-    throw result.value = e;
-  }
-
-  return result.value;
-}
-
 function makeCacheKey(args) {
   var parts = [];