Treat javascript internal errors as fatal

Spidermonkey sometimes throws an `InternalError` when exceeding memory limits,
when normally we'd expect it to crash or exit with a non-0 exit code. Because
we trap exceptions, and continue emitting rows, it is possible for users views
to randomly miss indexed rows based on whether GC had run or not, other
internal runtime state which may have been consuming more or less memory until
that time.

To prevent the view continuing processing documents, and randomly dropping
emitted rows, depending on memory pressure in the JS runtime at the time,
choose to treat Internal errors as fatal.

After an InternalError is raised we expect the process to exit just like it
would during OOM.

Add a test to assert this happens.

Fix https://github.com/apache/couchdb/issues/4504

3 files changed, 47 insertions, 1 deletions

diff --git a/share/server/loop.js b/share/server/loop.js
index 70a143a45..3ab303c21 100644
--- a/share/server/loop.js
+++ b/share/server/loop.js
@@ -130,6 +130,14 @@ var Loop = function() {
       quit(-1);
     } else if (type == "error") {
       respond(e);
+    } else if (e.name == "InternalError") {
+      // If the internal error is caught by handleViewError it will be
+      // re-thrown as a ["fatal", ...] error, and we already handle that above.
+      // Here we handle the case when the error is thrown outside of
+      // handleViewError, for instance when serializing the rows to be sent
+      // back to the user
+      respond(["error", e.name, e.message]);
+      quit(-1);
     } else if (e.error && e.reason) {
       // compatibility with old error format
       respond(["error", e.error, e.reason]);
diff --git a/share/server/views.js b/share/server/views.js
index 32d65e457..57cdaf3a9 100644
--- a/share/server/views.js
+++ b/share/server/views.js
@@ -73,6 +73,8 @@ var Views = (function() {
       // Throwing errors of the form ["fatal","error_key","reason"]
       // will kill the OS process. This is not normally what you want.
       throw(err);
+    } else if (err.name == "InternalError") {
+      throw(["fatal", err.name, err.message]);
     }
     var message = "function raised exception " + err.toSource();
     if (doc) message += " with doc._id " + doc._id;
diff --git a/src/couch/test/eunit/couch_js_tests.erl b/src/couch/test/eunit/couch_js_tests.erl
index 789b36321..afd42bb72 100644
--- a/src/couch/test/eunit/couch_js_tests.erl
+++ b/src/couch/test/eunit/couch_js_tests.erl
@@ -28,7 +28,8 @@ couch_js_test_() ->
                 fun should_roundtrip_modified_utf8/0,
                 fun should_replace_broken_utf16/0,
                 fun should_allow_js_string_mutations/0,
-                {timeout, 60000, fun should_exit_on_oom/0}
+                {timeout, 60000, fun should_exit_on_oom/0},
+                {timeout, 60000, fun should_exit_on_internal_error/0}
             ]
         }
     }.
@@ -236,6 +237,41 @@ should_exit_on_oom() ->
     true = couch_query_servers:proc_prompt(Proc, [<<"add_fun">>, Src]),
     trigger_oom(Proc).
 
+%% erlfmt-ignore
+should_exit_on_internal_error() ->
+    % A different way to trigger OOM which previously used to
+    % throw an InternalError on SM. Check that we still exit on that
+    % type of error
+    Src = <<"
+        function(doc) {
+            function mkstr(l) {
+                var r = 'r';
+                while (r.length < l) {r = r + r;}
+                return r;
+            }
+            var s = mkstr(96*1024*1024);
+            var a = [s,s,s,s,s,s,s,s];
+            var j = JSON.stringify(a);
+            emit(42, j.length);}
+    ">>,
+    Proc = couch_query_servers:get_os_process(<<"javascript">>),
+    true = couch_query_servers:proc_prompt(Proc, [<<"reset">>]),
+    true = couch_query_servers:proc_prompt(Proc, [<<"add_fun">>, Src]),
+    Doc = {[]},
+    try
+        couch_query_servers:proc_prompt(Proc, [<<"map_doc">>, Doc])
+    catch
+        % Expect either an internal error thrown if it catches it and
+        % emits an error log before dying
+        throw:{<<"InternalError">>, _} ->
+            ok;
+        % It may fail and just exit the process. That's expected as well
+        throw:{os_process_error, _} ->
+            ok
+    end,
+    % Expect the process to be dead
+    ?assertThrow({os_process_error, _}, couch_query_servers:proc_prompt(Proc, [<<"reset">>])).
+
 trigger_oom(Proc) ->
     Status =
         try