Skip to content

Commit e581401

Browse files
committed
Fix QuickJS scanner function_clause error
When users call list functions from maps it ends up confusing the map protocol and we get more emitted results than we have views. That results in a function_clause error emitted in the logs from lists:zip/2. Having seen enough of these cases let's add check for it, and if the view engines map results match but we end up with total emits no matching the view count, restart the js processes and continue on. While at it fix an erroneous copy-and-paste error in the comment of the random/date test.
1 parent f5b6066 commit e581401

File tree

2 files changed

+67
-13
lines changed

2 files changed

+67
-13
lines changed

src/couch_quickjs/src/couch_quickjs_scanner_plugin.erl

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,11 @@ views_validate(DDocId, #{?VIEWS := Views}, {Db, #st{} = St0}) when
258258
{Db, St}
259259
end
260260
catch
261+
throw:restart_procs ->
262+
#st{qjs_proc = Qjs, sm_proc = Sm} = St,
263+
proc_stop(Sm),
264+
proc_stop(Qjs),
265+
{Db, St#st{qjs_proc = undefined, sm_proc = undefined}};
261266
throw:{validate, Error} ->
262267
Meta = #{sid => SId, db => Db, ddoc => DDocId},
263268
validation_warning("view validation failed ~p", Error, Meta),
@@ -280,15 +285,26 @@ mapred_fold({Props = [_ | _]} = Doc, {ViewList = [_ | _], #st{} = St}) ->
280285
true -> ok;
281286
false -> throw({validate, {map_doc, DocId, QjsMapRes, SmMapRes}})
282287
end,
283-
case QjsMapRes of
284-
[_ | _] ->
285-
MapResZip = lists:zip(ViewList, QjsMapRes),
286-
ReduceKVs = lists:filtermap(fun reduce_filter_map/1, MapResZip),
287-
view_reduce_validate(St, ReduceKVs);
288-
_ ->
289-
ok
290-
end,
291-
{ViewList, St}.
288+
% Calling list functions from a map will result in a result list
289+
% longer than the number of views, so we match that the view list
290+
% and the results match
291+
case length(QjsMapRes) =:= length(ViewList) of
292+
true ->
293+
case QjsMapRes of
294+
[_ | _] ->
295+
MapResZip = lists:zip(ViewList, QjsMapRes),
296+
ReduceKVs = lists:filtermap(fun reduce_filter_map/1, MapResZip),
297+
view_reduce_validate(St, ReduceKVs);
298+
_ ->
299+
ok
300+
end,
301+
{ViewList, St};
302+
false ->
303+
% Exit early as calling list functions from a map leads to breaking the
304+
% query protocol. To avoid the messed up state affecting other views
305+
% exit early and restart the processes.
306+
throw(restart_procs)
307+
end.
292308

293309
reset_per_db_state(#st{qjs_proc = QjsProc, sm_proc = SmProc} = St) ->
294310
proc_stop(SmProc),
@@ -604,6 +620,8 @@ proc_reset(#proc{} = Proc) ->
604620

605621
% Proc utils
606622

623+
is_proc_alive(undefined) ->
624+
false;
607625
is_proc_alive(#proc{pid = Pid}) ->
608626
is_process_alive(Pid).
609627

src/couch_quickjs/test/couch_quickjs_scanner_plugin_tests.erl

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ couch_quickjs_scanner_plugin_test_() ->
2929
?TDEF_FE(t_filter_with_expected_error, 10),
3030
?TDEF_FE(t_empty_ddoc, 10),
3131
?TDEF_FE(t_multi_emit_map, 10),
32-
?TDEF_FE(t_non_deterministic_views, 10)
32+
?TDEF_FE(t_non_deterministic_views, 10),
33+
?TDEF_FE(t_handle_list_functions_in_maps, 10)
3334
]
3435
}.
3536

@@ -329,6 +330,26 @@ t_non_deterministic_views({_, DbName}) ->
329330
ok
330331
end.
331332

333+
t_handle_list_functions_in_maps({_, DbName}) ->
334+
ok = add_doc(DbName, ?DDOC1, ddoc_use_list_funs_in_maps(#{})),
335+
meck:reset(couch_scanner_server),
336+
meck:reset(?PLUGIN),
337+
config:set("couch_scanner_plugins", atom_to_list(?PLUGIN), "true", false),
338+
wait_exit(10000),
339+
?assertEqual(1, num_calls(start, 2)),
340+
case couch_server:with_spidermonkey() of
341+
true ->
342+
?assertEqual(1, num_calls(complete, 1)),
343+
?assert(num_calls(doc, 3) >= 5),
344+
?assertEqual(0, couch_stats:sample([couchdb, query_server, process_error_exits])),
345+
?assertEqual(0, couch_stats:sample([couchdb, query_server, process_errors])),
346+
?assertEqual(0, couch_stats:sample([couchdb, query_server, process_exits])),
347+
% start and complete = 2, no errors
348+
?assertEqual(2, log_calls(warning));
349+
false ->
350+
ok
351+
end.
352+
332353
reset_stats() ->
333354
Counters = [
334355
[couchdb, query_server, process_error_exits],
@@ -517,9 +538,7 @@ ddoc_view_multi_emit(Doc) ->
517538
}.
518539

519540
ddoc_view_non_determinism(Doc) ->
520-
% String.prototype.startsWith used as a differentiator between
521-
% SM and QuickJS. Make both emit items in different order. But at the
522-
% end of the day, that doesn't matter as those are sorted anyway
541+
% Test some functions with random and date values.
523542
Doc#{
524543
views => #{
525544
v1 => #{
@@ -537,3 +556,20 @@ ddoc_view_non_determinism(Doc) ->
537556
}
538557
}
539558
}.
559+
560+
ddoc_use_list_funs_in_maps(Doc) ->
561+
% If users call list functions from their maps, we used to crash
562+
% the scanner process with a function_clause.
563+
Doc#{
564+
views => #{
565+
v1 => #{
566+
map => <<
567+
"function(head, req) { \n"
568+
"var row;\n"
569+
"start({headers: {'Content-Type': 'text/plain'}});\n"
570+
"while(row = getRow()) {send('x'); send('y');}\n"
571+
"}"
572+
>>
573+
}
574+
}
575+
}.

0 commit comments

Comments
 (0)