erlef / rebar3_hex

Rebar3 Hex library
Apache License 2.0
101 stars 49 forks source link

Badarg error #192

Closed tsloughter closed 3 years ago

tsloughter commented 3 years ago

Well this is tough to debug.. At least with the latest rebar3 both the hex package of rebar3_hex and the latest master branch result in a crash when trying to publish:

Error: badarg
[]
tsloughter commented 3 years ago

Weird.. it worked when I used a rebar3 escript I had in my rebar3 dev dir but that should be the same commits as rebar3 3.14.2..

massemanet commented 3 years ago

after some ad hoc instrumentation I got this;

error,badarg,
                         [{erlang,apply,
                              [{r3_hex_http_httpc,#{profile => rebar}},
                               request,
                               [post,<<"https://hex.pm/api/keys">>,...]]}]
massemanet commented 3 years ago

I think this used to work...

Erlang/OTP 23 [erts-11.1.3]
1> Module = {lists, [[foo]]}.
{lists,[[foo]]}
2> Module:append([bar]).
** exception error: bad argument
     in function  apply/3
        called as apply({lists,[[foo]]},append,[[bar]])
tsloughter commented 3 years ago

Thanks. I think it is just a messed up match somewhere that is combining the module with the config that is supposed to be passed to it.

massemanet commented 3 years ago

Although it seems like the wrong place for a real fix, I worked around it in hex_http.erl;

request(Config, Method, URI, Headers, Body) when is_binary(URI) and is_map(Headers) ->
    {Adapter, AdapterConfig} = maps:get(http_adapter, Config),
    UserAgentFragment = maps:get(http_user_agent_fragment, Config),
    Headers2 = put_new(<<"user-agent">>, user_agent(UserAgentFragment), Headers),
    Adapter:request(Method, URI, Headers2, Body, AdapterConfig).
starbelly commented 3 years ago

Using a test project and a hex server running locally I am not able to replicate. @tsloughter or @massemanet can you share more details about the setup?

Edit: I wonder if the problem you're facing is related to baking in hex_core into the escript, this was done in the last release of rebar3.

tsloughter commented 3 years ago

Running with rebar3 from rebar3 local upgrade and latest rebar3_hex.

starbelly commented 3 years ago

@tsloughter What project is this you're trying to publish? I can try it locally to see if I can replicate.

tsloughter commented 3 years ago

I already published it. It was opentelemetry. The publish only fails when using the "unpacked" rebar3, I was able to publish when I used an escript.

Did you your local hex with the bin script that gets installed with rebar3 local upgrade?

starbelly commented 3 years ago

@tsloughter I built from the tag on the rebar3 from git repo. Let me try the other.

starbelly commented 3 years ago

Replicated... let me dig in now...

starbelly commented 3 years ago

What I have found is that upgrading hex_core in rebar3_hex to 0.7.1 resolves the issue, which is very strange since there's nothing significant in that release. I'll open a PR for this, but would like to understand this problem a little more first.

starbelly commented 3 years ago

Ok, I see what's going on @tsloughter. Per this commit https://github.com/hexpm/hex_core/commit/37d336c9b224b0139305f4886bbcc42cd9df7cb9 the adapter config changed. This is fine and rebar3 is setting this appropriately, but, if we're relying on rebar3 to set up the config and then trying to pass this into anything prior to v0.7.1, well that's when we get badarg. As prior to this version hex_core it just expecting http_adapter k/v in the config.

So the easy thing to do is just upgrade to 0.7.1. We might want to consider vendoring in hex_core so that we don't end up with these surprises. We might also not want to depend on rebar3 at all to set the config up. Thoughts?

Edit: We definitely need to be setting the adapter config ourselves as currently it will end up as r3_hex_httpc for the adapter and this might conflict with whatever version we happen to be pulling in.

tsloughter commented 3 years ago

Ah. Thanks for digging into it!

Handling configuration in rebar3_hex sounds like the right way to go if you think that'll be sufficient.

starbelly commented 3 years ago

It'll be sufficient, I'll get a PR opened soon.

starbelly commented 3 years ago

Noting that a new release ( v6.10.2 ) was cut and published to hex.pm after #193 was merged.