Improve errors for different publish contexts

src/rebar3_hex_publish.erl

@@ -51,6 +51,7 @@ do(State) ->
             ?RAISE(Reason)
     end.
 
+%% TODO: Move me somewhere else
 -spec format_error(any()) -> iolist().
 format_error(ErrList) when is_list(ErrList) ->
   F = fun(Err, Acc) ->
@@ -59,6 +60,27 @@ format_error(ErrList) when is_list(ErrList) ->
       end,
   More = "\n     Please see https://hex.pm/docs/rebar3_publish for more info.\n",
   lists:foldl(F, "Validator Errors:\n", ErrList) ++ More;
+
+format_error(no_write_key) ->
+    "No write key found for user. Be sure to authenticate first with:"
+    ++ " rebar3 hex user auth";
+
+%% Option errors
+format_error({app_not_found, AppName}) ->
+     io_lib:format("App ~s specified with --app switch not found in project", [AppName]);
+format_error({publish_package, app_switch_required}) ->
+    "--app required when publishing with the package argument in a umbrella";
+format_error({publish_docs, app_switch_required}) ->
+    "--app required when publishing with the docs argument in a umbrella";
+format_error({revert, app_switch_required}) ->
+    "--app required when reverting in a umbrella with multiple apps";
+format_error({required, repo}) ->
+    "publish requires a repo name argument to identify the repo to publish to";
+format_error({not_valid_repo, RepoName}) ->
+    io_lib:format("No configuration for repository ~ts found.", [RepoName]);
+
+
+%% Validation errors 
 format_error({validation_errors, Errs}) ->
     lists:map(fun(E) -> format_error(E) end, Errs);
 format_error({has_contributors, AppName}) ->
@@ -93,32 +115,16 @@ format_error({has_unstable_deps, Deps}) ->
     ],
     io_lib:format("~s~n~n~s~n~n~s~n", [MainMsg, DepList, Msg]);
 
-format_error({app_not_found, AppName}) ->
-     io_lib:format("App ~s specified with --app switch not found in project", [AppName]);
-format_error(bad_command) ->
-        "bad command";
+format_error({non_hex_deps, Excluded}) ->
+    Err = "Can not publish package because the following deps are not available"
+         ++ " in hex: ~s",
+    io_lib:format(Err, [string:join(Excluded, ", ")]);
+
+%% create package errors
 format_error({create_package, {error, Err}}) when is_list(Err) -> 
     io_lib:format("Error creating package : ~ts", [Err]);
-format_error({publish_package, app_switch_required}) ->
-    "--app required when publishing with the package argument in a umbrella";
-format_error({publish_docs, app_switch_required}) ->
-    "--app required when publishing with the docs argument in a umbrella";
-format_error({revert, app_switch_required}) ->
-    "--app required when reverting in a umbrella with multiple apps";
-format_error({required, repo}) ->
-    "publish requires a repo name argument to identify the repo to publish to";
-format_error({not_valid_repo, RepoName}) ->
-    io_lib:format("No configuration for repository ~ts found.", [RepoName]);
-format_error(no_write_key) ->
-    "No write key found for user. Be sure to authenticate first with:"
-    ++ " rebar3 hex user auth";
-format_error({publish, {error, {tarball, _} = Err}}) ->
-    hex_tarball:format_error(Err);
-format_error({publish, {error, #{<<"errors">> := Errors, <<"message">> := Message}}}) ->
-    ErrorString = errors_to_string(Errors),
-    io_lib:format("Failed to publish package: ~ts~n\t~ts", [Message, ErrorString]);
-format_error({publish, {error, #{<<"message">> := Message}}}) ->
-    io_lib:format("Failed to publish package: ~ts", [Message]);
+
+%% create docs errors
 format_error({create_docs, {error, {doc_provider_not_found, PrvName}}}) ->
    io_lib:format("The ~ts documentation provider could not be found", [PrvName]);
 format_error({create_docs, {error, {doc_provider_failed, PrvName}}}) ->
@@ -128,10 +134,38 @@ format_error({create_docs, {error, missing_doc_index}}) ->
     "If you provided --doc-dir option ensure that your docs were generated before running this task.\n"
     "Otherwise, check that your preferred doc provider is properly generating docs outside the scope of this task.\n"
     "Once resolved, run rebar3 hex publish docs <options> to publish only the docs for this version of your package.\n";
-format_error({non_hex_deps, Excluded}) ->
-    Err = "Can not publish package because the following deps are not available"
-         ++ " in hex: ~s",
-    io_lib:format(Err, [string:join(Excluded, ", ")]);
+
+%% publish package errors
+format_error({publish_package, Name, Version, {error, #{<<"errors">> := Errors, <<"message">> := Message}}}) ->
+    ErrorString = errors_to_string(Errors),
+    io_lib:format("Failed to publish package ~ts - ~ts : ~ts~n\t~ts", [Name, Version, Message, ErrorString]);
+format_error({publish_package, Name, Version, {error, #{<<"message">> := Message}}}) ->
+    io_lib:format("Failed to publish package ~ts - ~ts : ~ts", [Name, Version, Message]);
+
+%% publish docs errors
+format_error({publish_docs, Name, Version, {error, #{<<"errors">> := Errors, <<"message">> := Message}}}) ->
+    ErrorString = errors_to_string(Errors),
+    io_lib:format("Failed to publish docs for ~ts - ~ts : ~ts~n\t~ts", [Name, Version, Message, ErrorString]);
+format_error({publish_docs, Name, Version, {error, #{<<"message">> := Message}}}) ->
+    io_lib:format("Failed to publish docs for ~ts - ~ts : ~ts", [Name, Version, Message]);
+
+%% revert package errors
+format_error({revert_package, Name, Version, {error, #{<<"errors">> := Errors, <<"message">> := Message}}}) ->
+    ErrorString = errors_to_string(Errors),
+    io_lib:format("Failed to revert package ~ts - ~ts : ~ts~n\t~ts", [Name, Version, Message, ErrorString]);
+format_error({revert_package, Name, Version, {error, #{<<"message">> := Message}}}) ->
+    io_lib:format("Failed to revert package ~ts - ~ts : ~ts", [Name, Version, Message]);
+
+%% revert docs errors
+format_error({revert_docs, Name, Version, {error, #{<<"errors">> := Errors, <<"message">> := Message}}}) ->
+    ErrorString = errors_to_string(Errors),
+    io_lib:format("Failed to revert docs for ~ts - ~ts : ~ts~n\t~ts", [Name, Version, Message, ErrorString]);
+format_error({revert_docs, Name, Version, {error, #{<<"message">> := Message}}}) ->
+    io_lib:format("Failed to revert docs for ~ts - ~ts : ~ts", [Name, Version, Message]);
+
+
+%% TODO: Check if this is dead code 
+%% Server errors
 format_error(undefined_server_error) ->
     "Unknown server error";
 format_error({status, Status}) ->
@@ -144,6 +178,11 @@ format_error({status, Status, Error}) ->
   ErrorString = errors_to_string(Errors),
   Data =  [rebar3_hex_client:pretty_print_status(Status), Message, ErrorString],
   io_lib:format("Status Code: ~s~nHex Error: ~s~n\t~s", Data);
+
+%% TODO: Add helpful user error message
+format_error(bad_command) ->
+        "bad command";
+
 format_error(Reason) ->
     rebar3_hex_error:format_error(Reason).
 
@@ -279,7 +318,8 @@ publish_package(State, Repo, App, Args) ->
                     rebar_api:info("Published ~ts ~ts", [Name, Version]),
                     {ok, State};
                 Error ->
-                    ?RAISE({publish, Error})
+                    #{name := Name, version := Version} = Package,
+                    ?RAISE({publish_package, Name, Version, Error})
             end;
         abort ->
             rebar3_hex_io:say("Goodbye..."),
@@ -370,7 +410,7 @@ publish_docs(State, Repo, App, Args) ->
                     rebar_api:info("Published docs for ~ts ~ts", [Name, Vsn]),
                     {ok, State};
                 Reason ->
-                    ?RAISE({publish_docs, Reason})
+                    ?RAISE({publish_docs, Name, Vsn, Reason})
             end
     end.
 
@@ -404,7 +444,7 @@ revert_package(State, Repo, AppName, Vsn) ->
                     {ok, State}
             end;
         Reason ->
-            ?RAISE({revert, Reason})
+            ?RAISE({revert_package, BinAppName, BinVsn, Reason})
     end.
 
 revert_docs(State, Repo, AppName, Vsn) ->
@@ -417,7 +457,7 @@ revert_docs(State, Repo, AppName, Vsn) ->
             rebar_api:info("Successfully deleted docs for ~ts ~ts", [AppName, Vsn]),
             {ok, State};
         Reason ->
-            ?RAISE({revert, Reason})
+            ?RAISE({revert_docs, BinAppName, BinVsn, Reason})
     end.
 
 %% ===================================================================

test/rebar3_hex_integration_SUITE.erl

@@ -760,7 +760,9 @@ publish_unauthorized_test(Config) ->
     expects_repo_config(Setup),
     Exp = {error,
            {rebar3_hex_publish,
-            {publish,
+            {publish_package,
+             <<"valid">>,
+             "0.1.0",
              {error,
               #{<<"message">> =>
                 <<"account not authorized for this action">>}}}}},

test/rebar3_hex_publish_SUITE.erl

@@ -29,17 +29,33 @@ format_error_test(_Config) ->
         {{has_contributors, myapp}, <<"myapp.app.src : deprecated field contributors found">>},
         {no_write_key,
             <<"No write key found for user. Be sure to authenticate first with: rebar3 hex user auth">>},
+
+
         {
-            {publish,
+            {create_package, {error, "some error"}},
+            <<"Error creating package : some error">>
+        },
+
+        { 
+            {create_docs, {error, {doc_provider_not_found, foo}}},
+            <<"The foo documentation provider could not be found">>
+        },
+
+        { 
+            {create_docs, {error, {doc_provider_failed, foo}}},
+            <<"The foo documentation provider failed">>
+        },
+        {
+            {publish_package, <<"foo">>, <<"v1.0.0">>,
                 {error, #{
                     <<"message">> => <<"eh?">>,
                     <<"errors">> => [<<"Bad things">>, {<<"More bad">>, <<"things">>}]
                 }}},
-            <<"Failed to publish package: eh?\n\tBad thingsMore bad: things">>
+            <<"Failed to publish package foo - v1.0.0 : eh?\n\tBad thingsMore bad: things">>
         },
         {
-            {publish, {error, #{<<"message">> => "non sequitur"}}},
-            <<"Failed to publish package: non sequitur">>
+            {publish_package, <<"foo">>, <<"v1.0.0">>, {error, #{<<"message">> => "non sequitur"}}},
+            <<"Failed to publish package foo - v1.0.0 : non sequitur">>
         },
         {
             {publish_package, app_switch_required},
@@ -49,10 +65,46 @@ format_error_test(_Config) ->
             {publish_docs, app_switch_required},
             <<"--app required when publishing with the docs argument in a umbrella">>
         },
+        {
+            {publish_docs, <<"foo">>, <<"v1.0.0">>,
+                {error, #{
+                    <<"message">> => <<"eh?">>,
+                    <<"errors">> => [<<"Bad things">>, {<<"More bad">>, <<"things">>}]
+                }}},
+            <<"Failed to publish docs for foo - v1.0.0 : eh?\n\tBad thingsMore bad: things">>
+        },
+        {
+            {publish_docs, <<"foo">>, <<"v1.0.0">>, {error, #{<<"message">> => "non sequitur"}}},
+            <<"Failed to publish docs for foo - v1.0.0 : non sequitur">>
+        },
         {
             {revert, app_switch_required},
             <<"--app required when reverting in a umbrella with multiple apps">>
         },
+        {
+            {revert_package, <<"foo">>, <<"v1.0.0">>,
+                {error, #{
+                    <<"message">> => <<"eh?">>,
+                    <<"errors">> => [<<"Bad things">>, {<<"More bad">>, <<"things">>}]
+                }}},
+            <<"Failed to revert package foo - v1.0.0 : eh?\n\tBad thingsMore bad: things">>
+        },
+        {
+            {revert_package, <<"foo">>, <<"v1.0.0">>, {error, #{<<"message">> => "non sequitur"}}},
+            <<"Failed to revert package foo - v1.0.0 : non sequitur">>
+        },
+        {
+            {revert_docs, <<"foo">>, <<"v1.0.0">>,
+                {error, #{
+                    <<"message">> => <<"eh?">>,
+                    <<"errors">> => [<<"Bad things">>, {<<"More bad">>, <<"things">>}]
+                }}},
+            <<"Failed to revert docs for foo - v1.0.0 : eh?\n\tBad thingsMore bad: things">>
+        },
+        {
+            {revert_docs, <<"foo">>, <<"v1.0.0">>, {error, #{<<"message">> => "non sequitur"}}},
+            <<"Failed to revert docs for foo - v1.0.0 : non sequitur">>
+        },
         {
             {non_hex_deps, ["dep1", "dep2", "dep3"]},
             <<"Can not publish package because the following deps are not available in hex: dep1, dep2, dep3">>
@@ -85,7 +137,7 @@ format_error_test(_Config) ->
 
     lists:foreach(
         fun({Args, Exp}) ->
-            ?assertEqual(Exp, list_to_bitstring(rebar3_hex_publish:format_error(Args)))
+            ?assertEqual(Exp, list_to_binary(rebar3_hex_publish:format_error(Args)))
         end,
         ErrMap
     ),
@@ -127,9 +179,6 @@ format_error_test(_Config) ->
         rebar3_hex_publish:format_error({validation_errors, [{invalid_semver, {"eh", "42"}}]})
     ),
 
-    ?assertEqual(
-        "empty tarball", rebar3_hex_publish:format_error({publish, {error, {tarball, empty}}})
-    ),
     ?assertEqual(
         "An unknown error was encountered. Run with DEBUG=1 for more details.",
         rebar3_hex_publish:format_error(unknown)
@@ -149,7 +198,9 @@ format_error_test(_Config) ->
         ],
         10
     ],
-    ?assertEqual(ExpErr, rebar3_hex_publish:format_error({has_unstable_deps, [{"verl", "42"}]})).
+    ?assertEqual(ExpErr, rebar3_hex_publish:format_error({has_unstable_deps, [{"verl", "42"}]})),
+    ?assertMatch(<<"No index.html file was found", _Rest/binary>>, list_to_binary(rebar3_hex_publish:format_error({create_docs, {error,
+                                                                                                        missing_doc_index}}))).
 
 %%%%%%%%%%%%%%%
 %%% Helpers %%%