Skip to content

Commit e137b72

Browse files
nickvajanl
authored andcommitted
Restore the ability to return error object from maps
In views we drop any rows which throw errors. Sometimes users may want to either record which docs throw errors and build an error "key space" by doing `emit(['error', doc._id], e)` or something similar. This used to work in Spidermonkey 1.8.5, where `Error` objects were json stringified as objects with `message` and `name` fields. Newer Javascript engines like Spidermonkey 91 and QuickJS, however, stringify `Error` objects as `{}`. That's expected according to the Javascript standard, but is not as nice for users who want to debug their map functions. Users can of course still do `e.toString()` to stringify the error before emitting it, or emit the `.message` or `.name` separately, but since simply emitting or logging the error used to work, we can try bringing it back and let users to the same with the newer JS engines. Implementation-wise, the initial attempt of using `value instanceof Error` unexpecteldy failed, and Jan had solved the mystery - since `emit()` functions run in a sandbox, the `Error` object in the sandbox wasn't the same as the `Error` object in our emit wrapper code, so that always returned `false`. To solve that issue we opted to use some duck-typing, and see if the value is an object, is not null (`in` operator doesn't work on nulls), and then has the expected `Error` properties. The same thing applies to the `log()` function so we can try to do the check there. Thanks to Will Holley for noticing this issue and suggesting the solution, and to Jan Lehnardt for solving the `instanceof` sandbox mystery.
1 parent ab2e5c4 commit e137b72

File tree

3 files changed

+82
-4
lines changed

3 files changed

+82
-4
lines changed

share/server/util.js

+21-1
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,26 @@ function errstr(e) {
124124
return (e.toSource ? e.toSource() : e.toString());
125125
};
126126

127+
// If we have an object which looks like an Error, then make it so it
128+
// can be json stringified so it keeps the message and name,
129+
// otherwise, most modern JS engine will stringify Error object as
130+
// {}. Unfortnately, because of sandboxing we cannot use `e instanceof
131+
// Error` as the Error object in the sandbox won't technically be the
132+
// same error object as the one from our wrapper JS functions, so we
133+
// use some "ducktyping" to detect the Error.
134+
//
135+
function error_to_json(e) {
136+
if (typeof e === "object"
137+
&& e != null
138+
&& 'stack' in e
139+
&& 'name' in e
140+
&& 'message' in e
141+
) {
142+
return {'error': e.name, 'message': e.message}
143+
};
144+
return e;
145+
}
146+
127147
// prints the object as JSON, and rescues and logs any JSON.stringify() related errors
128148
function respond(obj) {
129149
try {
@@ -139,7 +159,7 @@ function log(message) {
139159
if (typeof message == "xml") {
140160
message = message.toXMLString();
141161
} else if (typeof message != "string") {
142-
message = JSON.stringify(message);
162+
message = JSON.stringify(error_to_json(message));
143163
}
144164
respond(["log", String(message)]);
145165
};

share/server/views.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ var Views = (function() {
8484
return {
8585
// view helper functions
8686
emit : function(key, value) {
87-
map_results.push([key, value]);
87+
map_results.push([key, error_to_json(value)]);
8888
},
8989
sum : function(values) {
9090
var rv = 0;

src/couch/test/eunit/couch_js_tests.erl

+60-2
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,20 @@ couch_js_test_() ->
1818
"Test couchjs",
1919
{
2020
setup,
21-
fun() -> test_util:start_couch([config]) end,
22-
fun test_util:stop_couch/1,
21+
fun() ->
22+
test_util:start_couch(),
23+
meck:new(couch_log, [passthrough])
24+
end,
25+
fun(Ctx) ->
26+
meck:unload(),
27+
test_util:stop_couch(Ctx)
28+
end,
2329
with([
2430
?TDEF(should_create_sandbox),
2531
?TDEF(should_reset_properly),
2632
?TDEF(should_freeze_doc_object),
33+
?TDEF(should_emit_error_details),
34+
?TDEF(should_log_error_details),
2735
?TDEF(should_roundtrip_utf8),
2836
?TDEF(should_roundtrip_modified_utf8),
2937
?TDEF(should_replace_broken_utf16),
@@ -103,6 +111,56 @@ should_freeze_doc_object(_) ->
103111
?assertEqual([[[null, 1041], [null, 1041]]], Result2),
104112
couch_query_servers:ret_os_process(Proc).
105113

114+
%% erlfmt-ignore
115+
should_emit_error_details(_) ->
116+
Src = <<"
117+
function(doc) {
118+
try {
119+
non_existent.fun_call
120+
} catch (e) {
121+
emit('err', e);
122+
}
123+
}
124+
">>,
125+
Proc = couch_query_servers:get_os_process(<<"javascript">>),
126+
true = prompt(Proc, [<<"add_fun">>, Src]),
127+
Result = prompt(Proc, [<<"map_doc">>, {[{<<"foo">>, 42}]}]),
128+
?assertMatch([[[<<"err">>, {[_ | _]}]]], Result),
129+
[[[<<"err">>, {ErrProps}]]] = Result,
130+
?assertEqual(<<"ReferenceError">>, couch_util:get_value(<<"error">>, ErrProps)),
131+
?assertMatch(<<_/binary>>, couch_util:get_value(<<"message">>, ErrProps)),
132+
couch_query_servers:ret_os_process(Proc).
133+
134+
%% erlfmt-ignore
135+
should_log_error_details(_) ->
136+
Src = <<"
137+
function(doc) {
138+
try {
139+
non_existent.fun_call
140+
} catch (e) {
141+
log(e);
142+
emit('err', 1);
143+
}
144+
}
145+
">>,
146+
Proc = couch_query_servers:get_os_process(<<"javascript">>),
147+
true = prompt(Proc, [<<"add_fun">>, Src]),
148+
meck:reset(couch_log),
149+
% Don't check too specifically just that we emitted ReferenceError somewhere
150+
% in there, otherwise each engine has its own error message format
151+
meck:expect(couch_log, info, fun
152+
("OS Process " ++ _, [_Port, <<Msg/binary>>]) ->
153+
?assertNotEqual(nomatch, binary:match(Msg, [<<"ReferenceError">>])),
154+
ok;
155+
(_, _) ->
156+
ok
157+
end),
158+
Result = prompt(Proc, [<<"map_doc">>, {[{<<"foo">>, 42}]}]),
159+
?assertEqual([[[<<"err">>, 1]]], Result),
160+
?assert(meck:num_calls(couch_log, info, 2) >= 1),
161+
meck:expect(couch_log, info, 2, meck:passthrough()),
162+
couch_query_servers:ret_os_process(Proc).
163+
106164
%% erlfmt-ignore
107165
should_roundtrip_utf8(_) ->
108166
% Try round tripping UTF-8 both directions through

0 commit comments

Comments
 (0)