From 14e8b50df3218b2f2e8a9279577129105459a578 Mon Sep 17 00:00:00 2001 From: Bryan Paxton Date: Fri, 24 Dec 2021 15:46:14 -0600 Subject: [PATCH] Improve errors for different publish contexts --- src/rebar3_hex_publish.erl | 110 ++++++++++++++++++-------- test/rebar3_hex_integration_SUITE.erl | 4 +- test/rebar3_hex_publish_SUITE.erl | 70 +++++++++++++--- 3 files changed, 138 insertions(+), 46 deletions(-) diff --git a/src/rebar3_hex_publish.erl b/src/rebar3_hex_publish.erl index afb38928..91fa21d9 100644 --- a/src/rebar3_hex_publish.erl +++ b/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({create_package, {error, Err}}) when is_list(Err) -> +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 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). @@ -277,14 +316,15 @@ publish_package(State, Repo, App, Args) -> #{dry_run := true} -> rebar_api:info("--dry-run enabled : will not publish package.", []), {ok, State}; - _ -> + _ -> case rebar3_hex_client:publish(Repo, Tarball, HexOpts) of {ok, _Res} -> #{name := Name, version := Version} = Package, rebar_api:info("Published ~ts ~ts", [Name, Version]), {ok, State}; - Error -> - ?RAISE({publish, Error}) + Error -> + #{name := Name, version := Version} = Package, + ?RAISE({publish_package, Name, Version, Error}) end end; abort -> @@ -376,7 +416,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. @@ -410,7 +450,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) -> @@ -423,7 +463,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. %% =================================================================== diff --git a/test/rebar3_hex_integration_SUITE.erl b/test/rebar3_hex_integration_SUITE.erl index 0b6223e7..a003052c 100644 --- a/test/rebar3_hex_integration_SUITE.erl +++ b/test/rebar3_hex_integration_SUITE.erl @@ -769,7 +769,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">>}}}}}, diff --git a/test/rebar3_hex_publish_SUITE.erl b/test/rebar3_hex_publish_SUITE.erl index d399fd74..8c82b659 100644 --- a/test/rebar3_hex_publish_SUITE.erl +++ b/test/rebar3_hex_publish_SUITE.erl @@ -19,7 +19,7 @@ format_error_test(_Config) -> {invalid_semver, {myapp, "-1.2.3"}}, <<"myapp.app.src : non-semantic version number \"-1.2.3\" found">> }, - { + { {invalid_semver_arg, <<"1.0">>}, <<"The version argument provided \"1.0\" is not a valid semantic version.">> }, @@ -29,17 +29,32 @@ 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">>}, + + + { + {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, + {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 +64,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 +136,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 +178,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 DIAGNOSTIC=1 for more details.", rebar3_hex_publish:format_error(unknown) @@ -149,7 +197,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 %%%