From 24f4974d31fbc0e2b3a0c637a1a222b3b6244f3e Mon Sep 17 00:00:00 2001 From: Bryan Paxton Date: Fri, 24 Dec 2021 14:21:06 -0600 Subject: [PATCH] Handle all doc and package creation errors - handle all possible errors returned from build - improve errors for different publish contexts - always set task to undefined when it's not defined --- src/rebar3_hex.erl | 6 +- src/rebar3_hex_build.erl | 41 +++++---- src/rebar3_hex_publish.erl | 126 +++++++++++++++++++------- test/rebar3_hex_integration_SUITE.erl | 4 +- test/rebar3_hex_publish_SUITE.erl | 72 ++++++++++++--- 5 files changed, 181 insertions(+), 68 deletions(-) diff --git a/src/rebar3_hex.erl b/src/rebar3_hex.erl index 7c0802de..8fcbf2c3 100644 --- a/src/rebar3_hex.erl +++ b/src/rebar3_hex.erl @@ -71,11 +71,13 @@ task_state(State) -> case rebar3_hex_config:repo(State) of {ok, Repo} -> Opts = get_args(State), + CurrentTask = maps:get(task, Opts, undefined), + Opts1 = Opts#{task => CurrentTask}, case rebar_state:project_apps(State) of [_App] = Apps -> - {ok, #{raw_opts => Opts0, raw_args => Args0, args => Opts, repo => Repo, state => State, multi_app => false, apps => Apps}}; + {ok, #{raw_opts => Opts0, raw_args => Args0, args => Opts1, repo => Repo, state => State, multi_app => false, apps => Apps}}; [_|_] = Apps -> - {ok, #{raw_opts => Opts0, raw_args => Args0, args => Opts, repo => Repo, state => State, multi_app => true, apps => Apps}} + {ok, #{raw_opts => Opts0, raw_args => Args0, args => Opts1, repo => Repo, state => State, multi_app => true, apps => Apps}} end; Err -> Err diff --git a/src/rebar3_hex_build.erl b/src/rebar3_hex_build.erl index d901507e..92418569 100644 --- a/src/rebar3_hex_build.erl +++ b/src/rebar3_hex_build.erl @@ -63,21 +63,24 @@ create_package(State, #{name := RepoName} = _Repo, App) -> | OptionalFiltered ]), - Tarball = create_package_tarball(Metadata, PackageFiles), - - Package = #{ - name => PkgName, - repo_name => RepoName, - deps => Deps1, - version => Version, - metadata => Metadata, - files => PackageFiles, - tarball => Tarball, - has_checkouts => has_checkouts(State) - }, - {ok, Package}; - {error, _} = Err -> - Err + case create_package_tarball(Metadata, PackageFiles) of + {error, _} = Err -> + Err; + Tarball -> + Package = #{ + name => PkgName, + repo_name => RepoName, + deps => Deps1, + version => Version, + metadata => Metadata, + files => PackageFiles, + tarball => Tarball, + has_checkouts => has_checkouts(State) + }, + {ok, Package} + end; + Error -> + Error end. update_versions(ConfigDeps, LockDeps) -> @@ -178,8 +181,8 @@ create_docs(State, Repo, App, Args) -> {ok, #{ tarball => Tarball, name => binarify(PkgName), vsn => binarify(Vsn) }}; - {error, _} = Err -> - Err; + {error, Reason} -> + {error, hex_tarball:format_error(Reason)}; Err -> Err end; @@ -268,8 +271,8 @@ create_package_tarball(Metadata, Files) -> case hex_tarball:create(Metadata, Files) of {ok, #{tarball := Tarball, inner_checksum := _Checksum}} -> Tarball; - {error, _} = Err -> - Err; + {error, Reason} -> + {error, hex_tarball:format_error(Reason)}; Error -> Error end. diff --git a/src/rebar3_hex_publish.erl b/src/rebar3_hex_publish.erl index b71f182d..d1b23296 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,36 +115,57 @@ 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({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]); -format_error({create_docs, {error, {doc_provider_not_found, PrvName}}}) -> - io_lib:format("The ~ts documentation provider could not be found", [PrvName]); 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]); + +%% 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}}}) -> + io_lib:format("The ~ts documentation provider failed", [PrvName]); +format_error({create_docs, {error, missing_doc_index}}) -> + "No index.html file was found in expected docs directory.\n" + "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"; + +%% 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}) -> @@ -135,6 +178,13 @@ 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); + +format_error(bad_command) -> + "Invalid arguments, expected one of:\n\n" + "rebar3 hex publish\n" + "rebar3 hex publish package\n" + "rebar3 hex publish docs\n"; + format_error(Reason) -> rebar3_hex_error:format_error(Reason). @@ -220,21 +270,24 @@ handle_task(#{args := #{revert := Vsn, app := AppName}, apps := Apps} = Task) -> %% Publish package and docs (the default path) %% =================================================================== -handle_task(#{args := #{app := undefined}, repo := Repo, state := State, apps := Apps} = Task) -> +handle_task(#{args := #{task := undefined, app := undefined}, repo := Repo, state := State, apps := Apps} = Task) -> #{args := Args} = Task, maybe_warn_about_single_app_args(Task), Selected = rebar3_hex_io:select_apps(Apps), lists:foreach(fun(App) -> publish(State, Repo, App, Args) end, Selected), {ok, State}; -handle_task(#{args := #{app := AppName}, apps := Apps, multi_app := true} = Task) -> +handle_task(#{args := #{task := undefined, app := AppName}, apps := Apps, multi_app := true} = Task) -> case rebar3_hex_app:find(Apps, AppName) of {error, app_not_found} -> ?RAISE({app_not_found, AppName}); {ok, App} -> #{args := Args, repo := Repo, state := State} = Task, publish(State, Repo, App, Args) - end. + end; + +handle_task(_) -> + ?RAISE(bad_command). -dialyzer({nowarn_function, publish/4}). publish(State, Repo, App, Args) -> @@ -268,14 +321,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 -> @@ -367,7 +421,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. @@ -375,6 +429,8 @@ create_docs(State, Repo, App, Args) -> case rebar3_hex_build:create_docs(State, Repo, App, Args) of {ok, Docs} -> Docs; + {error, no_doc_config} -> + rebar_api:warn("No documentation provider was configured, docs will not be generated.", []); Err -> ?RAISE({create_docs, Err}) end. @@ -399,7 +455,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) -> @@ -412,7 +468,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..40aaf17b 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 ), @@ -99,7 +150,7 @@ format_error_test(_Config) -> list_to_bitstring(rebar3_hex_publish:format_error(ErrList)) ), - ?assertEqual("bad command", rebar3_hex_publish:format_error(bad_command)), + ?assertMatch(<<"Invalid arguments", _/binary>>, list_to_binary(rebar3_hex_publish:format_error(bad_command))), ?assertEqual( "App foo specified with --app switch not found in project", rebar3_hex_publish:format_error({app_not_found, foo}) @@ -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 %%%