Fix crash when a dependency is missing app file

Patch up and add tests on #2112

3 files changed, 79 insertions, 3 deletions

diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl
index 0b7eeb1..0c15bb2 100644
--- a/src/rebar_app_discover.erl
+++ b/src/rebar_app_discover.erl
@@ -87,7 +87,9 @@ format_error({module_list, File}) ->
 format_error({missing_module, Module}) ->
     io_lib:format("Module defined in app file missing: ~p~n", [Module]);
 format_error({cannot_read_app_file, AppFile}) ->
-    io_lib:format("Cannot read app file: ~p~n", [AppFile]).
+    io_lib:format("Cannot read app file: ~p~n", [AppFile]);
+format_error({bad_term_file, _File, _Reason} = Error) ->
+    rebar_file_utils:format_error(Error).
 
 
 %% @doc merges configuration of a project app and the top level state
@@ -351,7 +353,7 @@ app_dir(AppFile) ->
 %% app file.
 -spec create_app_info(rebar_app_info:t(), file:name(), file:name()) -> rebar_app_info:t().
 create_app_info(AppInfo, AppDir, AppFile) ->
-    case rebar_config:consult_app_file(AppFile) of
+    try rebar_config:consult_app_file(AppFile) of
         [{application, AppName, AppDetails}] ->
             AppVsn = proplists:get_value(vsn, AppDetails),
             Applications = proplists:get_value(applications, AppDetails, []),
@@ -370,8 +372,11 @@ create_app_info(AppInfo, AppDir, AppFile) ->
                             false
                     end,
             rebar_app_info:dir(rebar_app_info:valid(AppInfo2, Valid), AppDir);
-        [] ->
+        _Invalid ->
             throw({error, {?MODULE, {cannot_read_app_file, AppFile}}})
+    catch
+        throw:{error, {rebar_file_utils, Err = {bad_term_file, _File, _Reason}}} ->
+            throw({error, {?MODULE, Err}}) % wrap this
     end.
 
 %% @doc Read in and parse the .app file if it is availabe. Do the same for
diff --git a/src/rebar_prv_app_discovery.erl b/src/rebar_prv_app_discovery.erl
index f5bab49..20b7f90 100644
--- a/src/rebar_prv_app_discovery.erl
+++ b/src/rebar_prv_app_discovery.erl
@@ -43,6 +43,8 @@ do(State) ->
             {error, {rebar_packages, Error}};
         throw:{error, {rebar_app_utils, Error}} ->
             {error, {rebar_app_utils, Error}};
+        throw:{error, {rebar_app_discover, Error}} ->
+            {error, {rebar_app_discover, Error}};
         throw:{error, Error} ->
             ?PRV_ERROR(Error)
     end.
diff --git a/test/rebar_discover_SUITE.erl b/test/rebar_discover_SUITE.erl
new file mode 100644
index 0000000..3fdd34b
--- /dev/null
+++ b/test/rebar_discover_SUITE.erl
@@ -0,0 +1,69 @@
+-module(rebar_discover_SUITE).
+-compile(export_all).
+
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+all() ->
+    [empty_app_src, bad_app_src, invalid_app_src].
+     %% note: invalid .app files without a .app.src also present
+     %% has rebar3 just ignoring the directory as not OTP-related.
+
+
+init_per_testcase(_, Config) ->
+    NewConfig = rebar_test_utils:init_rebar_state(Config, "discover_app_"),
+    AppDir = ?config(apps, NewConfig),
+
+    Name = rebar_test_utils:create_random_name("app1"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+
+    [{app_names, [Name]}, {vsns, [Vsn]}|NewConfig].
+
+end_per_testcase(_, Config) ->
+    Config.
+
+empty_app_src() ->
+    [{doc, "when there's an empty .app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+empty_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, ""),
+    ?assertEqual(
+       {error, {rebar_app_discover, {cannot_read_app_file, AppSrc}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+
+bad_app_src() ->
+    [{doc, "when there's a syntactically invalid "
+           ".app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+bad_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, "bad term file :("),
+    ?assertMatch(
+       {error, {rebar_app_discover, {bad_term_file, AppSrc, _}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+
+invalid_app_src() ->
+    [{doc, "when there's a syntactically valid but semantically invalid "
+           ".app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+invalid_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, "{applications, name_but_no_args}."),
+    ?assertEqual(
+       {error, {rebar_app_discover, {cannot_read_app_file, AppSrc}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+