Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions guides/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ These parameters can be specified in your *main* application (Eg the one you've

### Configure json_lib

One can configure which json library to use for encoding/decoding json structures. The module defined for this should expose two different functions:
Nova uses the OTP `json` module by default. You can override it by setting `json_lib` to any module that follows the same contract:

`encode(Structure) -> binary() | iolist()`
`encode(Structure) -> binary() | iodata()`

`decode(JsonString) -> {ok, Structure}`
`decode(Binary) -> Structure` (raises on invalid input)


## Handling errors in Nova
Expand Down
3 changes: 1 addition & 2 deletions rebar.config
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
{cowboy, "2.15.0"},
{erlydtl, "0.14.0"},
{jhn_stdlib, "5.11.2"},
{routing_tree, "1.0.11"},
{thoas, "1.2.1"}
{routing_tree, "1.0.11"}
]}.

{profiles, [
Expand Down
9 changes: 3 additions & 6 deletions rebar.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,20 @@
{<<"erlydtl">>,{pkg,<<"erlydtl">>,<<"0.14.0">>},0},
{<<"jhn_stdlib">>,{pkg,<<"jhn_stdlib">>,<<"5.11.2">>},0},
{<<"ranch">>,{pkg,<<"ranch">>,<<"2.2.0">>},1},
{<<"routing_tree">>,{pkg,<<"routing_tree">>,<<"1.0.11">>},0},
{<<"thoas">>,{pkg,<<"thoas">>,<<"1.2.1">>},0}]}.
{<<"routing_tree">>,{pkg,<<"routing_tree">>,<<"1.0.11">>},0}]}.
[
{pkg_hash,[
{<<"cowboy">>, <<"9CFE86ED7117BF045E10ADBEDB0170AF7BE57F2A3637E7BE143433D8DD267396">>},
{<<"cowlib">>, <<"318D385D55F657E9A5005838C4E426E13DCD724A691438384B6165A69687E531">>},
{<<"erlydtl">>, <<"964B2DC84F8C17ACFAA69C59BA129EF26AC45D2BA898C3C6AD9B5BDC8BA13CED">>},
{<<"jhn_stdlib">>, <<"785074F3CA368EAA8E9AF1592BC19AE9EF1F7AF30B2CD6456A6083173A8F5CCB">>},
{<<"ranch">>, <<"25528F82BC8D7C6152C57666CA99EC716510FE0925CB188172F41CE93117B1B0">>},
{<<"routing_tree">>, <<"72ACEF2095F0EC804F7AFD07EF781DDE5009425A1CA0A28F0706B1DB334A4812">>},
{<<"thoas">>, <<"19A25F31177A17E74004D4840F66D791D4298C5738790FA2CC73731EB911F195">>}]},
{<<"routing_tree">>, <<"72ACEF2095F0EC804F7AFD07EF781DDE5009425A1CA0A28F0706B1DB334A4812">>}]},
{pkg_hash_ext,[
{<<"cowboy">>, <<"179FB65140FB440A17B767AD53B755081506F9596C4DB5C49C0396D8C8643668">>},
{<<"cowlib">>, <<"58F1E425A9E04176F1D30E20116F57C4E90EF0E187552E9741C465BDF4044F70">>},
{<<"erlydtl">>, <<"D80EC044CD8F58809C19D29AC5605BE09E955040911B644505E31E9DD8143431">>},
{<<"jhn_stdlib">>, <<"2329CD16DEE46704AAB6184D09508E59DBA31C4D3255271DBB7D34D115ECA508">>},
{<<"ranch">>, <<"FA0B99A1780C80218A4197A59EA8D3BDAE32FBFF7E88527D7D8A4787EFF4F8E7">>},
{<<"routing_tree">>, <<"85982C7AC502892C5179CD2A591331003BACD2D2A71723640BA7D23F45408E6E">>},
{<<"thoas">>, <<"E38697EDFFD6E91BD12CEA41B155115282630075C2A727E7A6B2947F5408B86A">>}]}
{<<"routing_tree">>, <<"85982C7AC502892C5179CD2A591331003BACD2D2A71723640BA7D23F45408E6E">>}]}
].
6 changes: 3 additions & 3 deletions src/controllers/nova_error_controller.erl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ not_found(Req) ->
lists:member(<<"text/html">>, AcceptList)} of
{true, _} ->
%% Render a json response
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
Json = JsonLib:encode(#{message => "Resource not found"}),
{status, 404, #{<<"content-type">> => <<"application/json">>}, Json};
{_, true} ->
Expand All @@ -47,7 +47,7 @@ server_error(#{crash_info := #{status_code := StatusCode} = CrashInfo} = Req) ->
true ->
case cowboy_req:header(<<"accept">>, Req) of
<<"application/json">> ->
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
Json = JsonLib:encode(Variables),
{status, StatusCode, #{<<"content-type">> => <<"application/json">>}, Json};
<<"text/html">> ->
Expand All @@ -72,7 +72,7 @@ server_error(#{crash_info := #{class := Class, reason := Reason}} = Req) ->
%% We do show a proper error response
case cowboy_req:header(<<"accept">>, Req) of
<<"application/json">> ->
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
Json = JsonLib:encode(Variables),
{status, 500, #{<<"content-type">> => <<"application/json">>}, Json};
<<"text/html">> ->
Expand Down
3 changes: 1 addition & 2 deletions src/nova.app.src
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
compiler,
erlydtl,
jhn_stdlib,
routing_tree,
thoas
routing_tree
]},
{env,[]},
{modules,[nova]},
Expand Down
4 changes: 2 additions & 2 deletions src/nova_basic_handler.erl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
handle_json({json, StatusCode, Headers, Req0, JSON}, Callback, _Req) ->
handle_json({json, StatusCode, Headers, JSON}, Callback, Req0);
handle_json({json, StatusCode, Headers, JSON}, _Callback, Req) ->
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
EncodedJSON = JsonLib:encode(JSON),
Headers0 = maps:merge(#{<<"content-type">> => <<"application/json">>}, Headers),
Req0 = cowboy_req:set_resp_headers(Headers0, Req),
Expand Down Expand Up @@ -154,7 +154,7 @@ handle_status({status, Status, ExtraHeaders, JSON, Req0}, Callback, _Req) ->
handle_status({status, Status, ExtraHeaders, JSON}, Callback, Req0);
handle_status({status, Status, ExtraHeaders, JSON}, _Callback, Req) when is_map(JSON) ->
%% We do not need to render a status page since we just return a JSON structure
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
Headers0 = maps:merge(#{<<"content-type">> => <<"application/json">>}, ExtraHeaders),
Req0 = cowboy_req:set_resp_headers(Headers0, Req),
Req1 = Req0#{resp_status_code => Status},
Expand Down
56 changes: 25 additions & 31 deletions src/nova_jsonlogger.erl
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ merge_meta(Msg, Meta0, Config) ->
maps:merge(Msg, Meta2).

encode(Data, Config) ->
JsonLib = nova:get_env(json_lib, thoas),
JsonLib = nova:get_env(json_lib, json),
Json = JsonLib:encode(Data),
case new_line(Config) of
true -> [Json, new_line_type(Config)];
Expand Down Expand Up @@ -162,7 +162,8 @@ meta_with(Meta, _ConfigNotPresent) ->
-include_lib("eunit/include/eunit.hrl").

-define(assertJSONEqual(Expected, Actual),
?assertEqual(thoas:decode(Expected), thoas:decode(Actual))
?assertEqual(json:decode(iolist_to_binary(Expected)),
json:decode(iolist_to_binary(Actual)))
).

format_test() ->
Expand Down Expand Up @@ -260,20 +261,20 @@ meta_without_test() ->
meta => #{secret => xyz}
},
?assertEqual(
{ok, #{
#{
<<"answer">> => 42,
<<"level">> => <<"info">>,
<<"secret">> => <<"xyz">>
}},
thoas:decode(format(Error, #{}))
},
json:decode(iolist_to_binary(format(Error, #{})))
),
Config2 = #{meta_without => [secret]},
?assertEqual(
{ok, #{
#{
<<"answer">> => 42,
<<"level">> => <<"info">>
}},
thoas:decode(format(Error, Config2))
},
json:decode(iolist_to_binary(format(Error, Config2)))
),
ok.

Expand All @@ -284,59 +285,52 @@ meta_with_test() ->
meta => #{secret => xyz}
},
?assertEqual(
{ok, #{
#{
<<"answer">> => 42,
<<"level">> => <<"info">>,
<<"secret">> => <<"xyz">>
}},
thoas:decode(format(Error, #{}))
},
json:decode(iolist_to_binary(format(Error, #{})))
),
Config2 = #{meta_with => [level]},
?assertEqual(
{ok, #{
#{
<<"answer">> => 42,
<<"level">> => <<"info">>
}},
thoas:decode(format(Error, Config2))
},
json:decode(iolist_to_binary(format(Error, Config2)))
),
ok.

newline_test() ->
ConfigDefault = #{new_line => true},
?assertEqual(
[<<"{\"level\":\"alert\",\"text\":\"derp\"}">>, <<"\n">>],
format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigDefault)
<<"{\"level\":\"alert\",\"text\":\"derp\"}\n">>,
iolist_to_binary(format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigDefault))
),
ConfigCRLF = #{
new_line_type => crlf,
new_line => true
},
?assertEqual(
[<<"{\"level\":\"alert\",\"text\":\"derp\"}">>, <<"\r\n">>],
format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigCRLF)
<<"{\"level\":\"alert\",\"text\":\"derp\"}\r\n">>,
iolist_to_binary(format(#{level => alert, msg => {string, "derp"}, meta => #{}}, ConfigCRLF))
).

newline_types_test() ->
Base = #{level => alert, msg => {string, "x"}, meta => #{}},
%% nl
?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true, new_line_type => nl}))),
%% unix
?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true, new_line_type => unix}))),
%% crlf
?assertEqual(<<"\r\n">>, lists:last(format(Base, #{new_line => true, new_line_type => crlf}))),
%% windows
?assertEqual(<<"\r\n">>, lists:last(format(Base, #{new_line => true, new_line_type => windows}))),
%% cr
?assertEqual(<<"\r">>, lists:last(format(Base, #{new_line => true, new_line_type => cr}))),
%% macos9
?assertEqual(<<"\r">>, lists:last(format(Base, #{new_line => true, new_line_type => macos9}))),
%% default (no new_line_type specified)
?assertEqual(<<"\n">>, lists:last(format(Base, #{new_line => true}))).

no_newline_test() ->
Base = #{level => alert, msg => {string, "x"}, meta => #{}},
Result = format(Base, #{}),
?assert(is_binary(Result)).
?assert(is_binary(iolist_to_binary(Result))).

jsonify_types_test() ->
%% atom
Expand Down Expand Up @@ -365,29 +359,29 @@ jsonify_types_test() ->

format_keyval_list_test() ->
Event = #{level => info, msg => {report, [{key, <<"val">>}]}, meta => #{}},
{ok, Decoded} = thoas:decode(format(Event, #{})),
Decoded = json:decode(iolist_to_binary(format(Event, #{}))),
?assertEqual(<<"val">>, maps:get(<<"key">>, Decoded)).

format_format_terms_test() ->
Event = #{level => info, msg => {"hello ~s", ["world"]}, meta => #{}},
{ok, Decoded} = thoas:decode(format(Event, #{})),
Decoded = json:decode(iolist_to_binary(format(Event, #{}))),
?assertEqual(<<"hello world">>, maps:get(<<"text">>, Decoded)).

format_error_logger_test() ->
Event = #{level => error,
msg => {report, #{format => "error: ~p", args => [bad], label => {error_logger, error}}},
meta => #{}},
{ok, Decoded} = thoas:decode(format(Event, #{})),
Decoded = json:decode(iolist_to_binary(format(Event, #{}))),
?assert(maps:is_key(<<"text">>, Decoded)).

nested_map_test() ->
Event = #{level => info, msg => {report, #{outer => #{inner => value}}}, meta => #{}},
{ok, Decoded} = thoas:decode(format(Event, #{})),
Decoded = json:decode(iolist_to_binary(format(Event, #{}))),
?assertEqual(#{<<"inner">> => <<"value">>}, maps:get(<<"outer">>, Decoded)).

list_of_maps_test() ->
Event = #{level => info, msg => {report, #{items => [#{a => 1}, #{b => 2}]}}, meta => #{}},
{ok, Decoded} = thoas:decode(format(Event, #{})),
Decoded = json:decode(iolist_to_binary(format(Event, #{}))),
?assertEqual([#{<<"a">> => 1}, #{<<"b">> => 2}], maps:get(<<"items">>, Decoded)).

system_time_to_iso8601_test() ->
Expand Down
13 changes: 7 additions & 6 deletions src/plugins/nova_request_plugin.erl
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,16 @@ modulate_state(Req = #{headers := #{<<"content-type">> := <<"application/json",
{stop, Req400, State};
modulate_state(Req = #{headers := #{<<"content-type">> := <<"application/json", _/binary>>}, body := Body}, [{decode_json_body, true}|Tl], State) ->
%% Decode the data
JsonLib = nova:get_env(json_lib, thoas),
case JsonLib:decode(Body) of
{ok, JSON} ->
modulate_state(Req#{json => JSON}, Tl, State);
Error ->
JsonLib = nova:get_env(json_lib, json),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add my comment to this block since it showcase my point;

Here we do try...catch but in the rest of the code we just let it crash. That we can not have - we need to discuss a strategy on how to handle these kinds of problems. Do we crash or do we catch it? It might be that we want to do different things in different situations - but think we need to discuss this more broadly.

try JsonLib:decode(Body) of
JSON ->
modulate_state(Req#{json => JSON}, Tl, State)
catch
Class:Reason ->
Req400 = cowboy_req:reply(400, Req),
logger:warning(#{status_code => 400,
msg => "Failed to decode json.",
error => Error}),
error => {Class, Reason}}),
{stop, Req400, State}
end;
modulate_state(#{headers := #{<<"content-type">> := <<"application/x-www-form-urlencoded", _/binary>>}, body := Body} = Req,
Expand Down
2 changes: 1 addition & 1 deletion test/nova_error_controller_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ not_found_json_accept_test_() ->
Req1 = nova_test_helper:with_header(<<"accept">>, <<"application/json">>, Req),
{status, 404, Headers, Body} = nova_error_controller:not_found(Req1),
?assertEqual(<<"application/json">>, maps:get(<<"content-type">>, Headers)),
?assert(is_binary(Body))
?assert(is_binary(iolist_to_binary(Body)))
end}.

%% When no accept header, defaults to JSON
Expand Down
4 changes: 2 additions & 2 deletions test/nova_test_helper.erl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ with_header(Name, Value, Req = #{headers := Headers}) ->

-spec with_json_body(map() | binary(), map()) -> map().
with_json_body(JSON, Req) when is_map(JSON) ->
Body = thoas:encode(JSON),
Body = iolist_to_binary(json:encode(JSON)),
with_json_body(Body, Req);
with_json_body(Body, Req) when is_binary(Body) ->
Req1 = with_content_type(<<"application/json">>, Req),
Expand Down Expand Up @@ -61,7 +61,7 @@ setup_nova_env() ->
Prev = application:get_env(nova, bootstrap_application),
application:set_env(nova, bootstrap_application, nova),
application:set_env(nova, environment, dev),
application:set_env(nova, json_lib, thoas),
application:set_env(nova, json_lib, json),
Prev.

-spec cleanup_nova_env(undefined | {ok, atom()}) -> ok.
Expand Down
Loading