diff --git a/src/decoders/router_decoder_custom_worker.erl b/src/decoders/router_decoder_custom_worker.erl index 05dd91f3c..474dff4a1 100644 --- a/src/decoders/router_decoder_custom_worker.erl +++ b/src/decoders/router_decoder_custom_worker.erl @@ -105,13 +105,32 @@ handle_call( ) of {error, invalid_context} -> + %% Execution reaching here implies V8 vm restarted recently {Delay, Backoff1} = backoff:fail(Backoff0), + %% Refresh their Context and JS definition: _ = erlang:send_after(Delay, self(), ?INIT_CONTEXT), {reply, {error, invalid_context}, State1#state{ context = undefined, backoff = Backoff1 }}; - {error, _} = Error -> + {error, Reason0} = Error -> + %% Track repeat offenders of bad JS code via external monitoring + Reason1 = + case size(Reason0) of + N when N =< 10 -> Reason0; + _ -> binary:part(Reason0, 0, 10) + end, + lager:error( + "V8 call error=\"~p\" device_id=~p app_eui=~p dev_eui=~p", + [ + Reason1, + maps:get(device_id, UplinkDetails, unknown), + maps:get(app_eui, UplinkDetails, unknown), + maps:get(dev_eui, UplinkDetails, unknown) + ] + ), + %% Due to limited error granularity from V8 Port, restart after any error: + erlang_v8:restart_vm(VM), {reply, Error, State1}; {ok, _} = OK -> {reply, OK, State1} diff --git a/test/router_decoder_SUITE.erl b/test/router_decoder_SUITE.erl index d1a339fef..928a26a96 100644 --- a/test/router_decoder_SUITE.erl +++ b/test/router_decoder_SUITE.erl @@ -10,7 +10,8 @@ decode_test/1, template_test/1, timeout_test/1, - too_many_test/1 + too_many_test/1, + v8_recovery_test/1 ]). -include_lib("common_test/include/ct.hrl"). @@ -348,6 +349,35 @@ too_many_test(Config) -> ?assertEqual(1, ets:info(router_decoder_custom_sup_ets, size)), ok. +v8_recovery_test(Config) -> + %% Set console to decoder channel mode + Tab = proplists:get_value(ets, Config), + ets:insert(Tab, {channel_type, decoder}), + + test_utils:join_device(Config), + + %% Waiting for reply from router to hotspot + test_utils:wait_state_channel_message(1250), + + ValidDecoder = <<"function Decoder(a,b,c) { return 42; }">>, + Result = 42, + BogusDecoder = <<"function Decoder(a,b,c) { crash @ here! }">>, + Port = 6, + Payload = erlang:binary_to_list(base64:decode(<<"H4Av/xACRU4=">>)), + UplinkDetails = #{}, + + %% One context sends a valid function definiton and another sends + %% bogus javascript, causing the V8 VM to be restarted. The first + %% context should then recover. + + {ok, Pid} = router_decoder_custom_sup:add(ValidDecoder), + {ok, Result} = router_decoder_custom_worker:decode(Pid, Payload, Port, UplinkDetails), + + {error, _} = router_decoder_custom_sup:add(BogusDecoder), + + {ok, Result} = router_decoder_custom_worker:decode(Pid, Payload, Port, UplinkDetails), + ok. + %% ------------------------------------------------------------------ %% Helper functions %% ------------------------------------------------------------------ diff --git a/test/router_v8_SUITE.erl b/test/router_v8_SUITE.erl new file mode 100644 index 000000000..54ad11a73 --- /dev/null +++ b/test/router_v8_SUITE.erl @@ -0,0 +1,100 @@ +-module(router_v8_SUITE). + +-export([ + all/0, + init_per_testcase/2, + end_per_testcase/2 +]). + +-export([ + invalid_context_test/1 +]). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +%%-------------------------------------------------------------------- +%% @public +%% @doc +%% Running tests for this suite +%% @end +%%-------------------------------------------------------------------- +all() -> + [invalid_context_test]. + +%%-------------------------------------------------------------------- +%% TEST CASE SETUP +%%-------------------------------------------------------------------- +init_per_testcase(TestCase, Config) -> + test_utils:init_per_testcase(TestCase, Config). + +%%-------------------------------------------------------------------- +%% TEST CASE TEARDOWN +%%-------------------------------------------------------------------- +end_per_testcase(TestCase, Config) -> + test_utils:end_per_testcase(TestCase, Config). + +%%-------------------------------------------------------------------- +%% TEST CASES +%%-------------------------------------------------------------------- + +invalid_context_test(_Config) -> + GoodFunction = + <<"function Decoder(bytes, port) {\n" + " var payload = {\"Testing\": \"42\"};\n" + " return payload;\n" + "}">>, + BadFunction = <<"function Decoder() { returrrrrrrn 0; }">>, + + VMPid = + case router_v8:start_link(#{}) of + {ok, Pid} -> Pid; + {error, {already_started, Pid}} -> Pid + end, + {ok, VM} = router_v8:get(), + {ok, Context1} = erlang_v8:create_context(VM), + {ok, Context2} = erlang_v8:create_context(VM), + ?assertNotMatch(Context1, Context2), + + Payload = erlang:binary_to_list(base64:decode(<<"H4Av/xACRU4=">>)), + Port = 6, + Result = #{<<"Testing">> => <<"42">>}, + + %% Eval good function and ensure function works more than once + ?assertMatch({ok, undefined}, erlang_v8:eval(VM, Context1, GoodFunction)), + ?assertMatch({ok, Result}, erlang_v8:call(VM, Context1, <<"Decoder">>, [Payload, Port])), + ?assertMatch({ok, Result}, erlang_v8:call(VM, Context1, <<"Decoder">>, [Payload, Port])), + + %% Call undefined function + ?assertMatch( + {error, <<"ReferenceError: Decoder is not defined", _/binary>>}, + erlang_v8:call(VM, Context2, <<"Decoder">>, [Payload, Port]) + ), + + %% First Context still works + ?assertMatch({ok, Result}, erlang_v8:call(VM, Context1, <<"Decoder">>, [Payload, Port])), + + %% Eval bad function + ?assertMatch({error, crashed}, erlang_v8:eval(VM, Context2, BadFunction)), + + %% Upon an error (other than invalid_source_size), v8 Port gets + %% killed and restarted + ?assertMatch(true, erlang:is_process_alive(VMPid)), + + %% Unintuitively, we get invalid context when attempting to reuse first Context: + ?assertMatch( + {error, invalid_context}, + erlang_v8:call(VM, Context1, <<"Decoder">>, [Payload, Port]) + ), + %% But that's because set of Context needs to be repopulated within V8's VM: + erlang_v8:restart_vm(VM), + + %% First Context no longer works-- not ideal behavior but what we + %% have at hand. Ideally, return value would still match `Result'. + ?assertMatch( + {error, invalid_context}, + erlang_v8:call(VM, Context1, <<"Decoder">>, [Payload, Port]) + ), + + gen_server:stop(VMPid), + ok.