Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix dialyzer warnings #12

Merged
merged 2 commits into from

2 participants

@fdmanana

Also added missing possible return values for the
request/4,5,6,9 functions. Those were the values
that can be returned when the partial_download and
partial_upload options are used.

fdmanana added some commits
@fdmanana fdmanana Fix dialyzer warnings
Also added missing possible return values for the
request/4,5,6,9 functions. Those were the values
that can be returned when the partial_download and
partial_upload options are used.
b4b0abc
@fdmanana fdmanana Simpler and consistent error handling
Don't raise an error if the http client process died
with a reason other than 'timeout'. Instead return a
term with the shape {error, any()}.
This was missed in the following pull request:

esl#10
74a4cba
@paulgray paulgray merged commit 89d5b49 into from
@t3rm1n4l t3rm1n4l referenced this pull request from a commit in t3rm1n4l/couchdb
@fdmanana fdmanana Update lhttpc to latest version
This simplifies error handling and fixes many
dialyzer warnings. Related upstream pull requests
are:

* esl/lhttpc#10
* esl/lhttpc#11
* esl/lhttpc#12

This is part of CBD-73

Change-Id: I5c7239ee67cbb1fa5b88fd51e9ce803e9b244f6a
Reviewed-on: http://review.couchbase.org/15099
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Volker Mische <volker.mische@gmail.com>
eeff685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 20, 2012
  1. @fdmanana

    Fix dialyzer warnings

    fdmanana authored
    Also added missing possible return values for the
    request/4,5,6,9 functions. Those were the values
    that can be returned when the partial_download and
    partial_upload options are used.
  2. @fdmanana

    Simpler and consistent error handling

    fdmanana authored
    Don't raise an error if the http client process died
    with a reason other than 'timeout'. Instead return a
    term with the shape {error, any()}.
    This was missed in the following pull request:
    
    esl#10
This page is out of date. Refresh to see the latest.
Showing with 77 additions and 70 deletions.
  1. +58 −66 src/lhttpc.erl
  2. +13 −1 src/lhttpc_types.hrl
  3. +6 −3 test/lhttpc_tests.erl
View
124 src/lhttpc.erl
@@ -46,8 +46,15 @@
-include("lhttpc_types.hrl").
-include("lhttpc.hrl").
--type result() :: {ok, {{pos_integer(), string()}, headers(), binary()}} |
- {error, atom()}.
+-type upload_state() :: {pid(), window_size()}.
+-type body() :: binary() |
+ 'undefined' | % HEAD request.
+ pid(). % When partial_download option is used.
+
+-type result() ::
+ {ok, {{pos_integer(), string()}, headers(), body()}} |
+ {ok, upload_state()} |
+ {error, atom()}.
%% @hidden
-spec start(normal | {takeover, node()} | {failover, node()}, any()) ->
@@ -346,7 +353,7 @@ request(URL, Method, Hdrs, Body, Timeout, Options) ->
-spec request(string(), 1..65535, true | false, string(), atom() | string(),
headers(), iolist(), pos_integer(), [option()]) -> result().
request(Host, Port, Ssl, Path, Method, Hdrs, Body, Timeout, Options) ->
- verify_options(Options, []),
+ verify_options(Options),
Args = [self(), Host, Port, Ssl, Path, Method, Hdrs, Body, Options],
Pid = spawn_link(lhttpc_client, request, Args),
receive
@@ -373,8 +380,7 @@ request(Host, Port, Ssl, Path, Method, Hdrs, Body, Timeout, Options) ->
%% Would be the same as calling
%% `send_body_part(UploadState, BodyPart, infinity)'.
%% @end
--spec send_body_part({pid(), window_size()}, iolist()) ->
- {pid(), window_size()} | result().
+-spec send_body_part(upload_state(), iolist() | 'http_eob') -> result().
send_body_part({Pid, Window}, IoList) ->
send_body_part({Pid, Window}, IoList, infinity).
@@ -399,8 +405,7 @@ send_body_part({Pid, Window}, IoList) ->
%% there is no response within `Timeout' milliseconds, the request is
%% canceled and `{error, timeout}' is returned.
%% @end
--spec send_body_part({pid(), window_size()}, iolist(), timeout()) ->
- {ok, {pid(), window_size()}} | result().
+-spec send_body_part(upload_state(), iolist() | 'http_eob', timeout()) -> result().
send_body_part({Pid, _Window}, http_eob, Timeout) when is_pid(Pid) ->
Pid ! {body_part, self(), http_eob},
read_response(Pid, Timeout);
@@ -485,7 +490,9 @@ send_trailers({Pid, _Window}, Trailers, Timeout)
%% Would be the same as calling
%% `get_body_part(HTTPClient, infinity)'.
%% @end
--spec get_body_part(pid()) -> {ok, binary()} | {ok, {http_eob, headers()}}.
+-spec get_body_part(pid()) -> {ok, binary()} |
+ {ok, {http_eob, headers()}} |
+ {error, term()}.
get_body_part(Pid) ->
get_body_part(Pid, infinity).
@@ -503,7 +510,9 @@ get_body_part(Pid) ->
%% response those are returned with `http_eob' as well.
%% @end
-spec get_body_part(pid(), timeout()) ->
- {ok, binary()} | {ok, {http_eob, headers()}}.
+ {ok, binary()} |
+ {ok, {http_eob, headers()}} |
+ {error, term()}.
get_body_part(Pid, Timeout) ->
receive
{body_part, Pid, Bin} ->
@@ -538,70 +547,53 @@ kill_client(Pid) ->
{response, Pid, R} ->
erlang:demonitor(Monitor, [flush]),
R;
- {'DOWN', _, process, Pid, timeout} ->
- {error, timeout};
{'DOWN', _, process, Pid, Reason} ->
- erlang:error(Reason)
+ {error, Reason}
end.
--spec verify_options(options(), options()) -> ok.
-verify_options([{send_retry, N} | Options], Errors)
- when is_integer(N), N >= 0 ->
- verify_options(Options, Errors);
-verify_options([{connect_timeout, infinity} | Options], Errors) ->
- verify_options(Options, Errors);
-verify_options([{connect_timeout, MS} | Options], Errors)
+-spec verify_options(options()) -> ok.
+verify_options([{send_retry, N} | Options]) when is_integer(N), N >= 0 ->
+ verify_options(Options);
+verify_options([{connect_timeout, infinity} | Options]) ->
+ verify_options(Options);
+verify_options([{connect_timeout, MS} | Options])
when is_integer(MS), MS >= 0 ->
- verify_options(Options, Errors);
-verify_options([{partial_upload, WindowSize} | Options], Errors)
+ verify_options(Options);
+verify_options([{partial_upload, WindowSize} | Options])
when is_integer(WindowSize), WindowSize >= 0 ->
- verify_options(Options, Errors);
-verify_options([{partial_upload, infinity} | Options], Errors) ->
- verify_options(Options, Errors);
-verify_options([{partial_download, DownloadOptions} | Options], Errors)
+ verify_options(Options);
+verify_options([{partial_upload, infinity} | Options]) ->
+ verify_options(Options);
+verify_options([{partial_download, DownloadOptions} | Options])
when is_list(DownloadOptions) ->
- case verify_partial_download(DownloadOptions, []) of
- [] ->
- verify_options(Options, Errors);
- OptionErrors ->
- NewErrors = [{partial_download, OptionErrors} | Errors],
- verify_options(Options, NewErrors)
- end;
-verify_options([{connect_options, List} | Options], Errors)
- when is_list(List) ->
- verify_options(Options, Errors);
-verify_options([{proxy, List} | Options], Errors)
- when is_list(List) ->
- verify_options(Options, Errors);
-verify_options([{proxy_ssl_options, List} | Options], Errors)
- when is_list(List) ->
- verify_options(Options, Errors);
-verify_options([{pool, PidOrName} | Options], Errors)
+ verify_partial_download(DownloadOptions),
+ verify_options(Options);
+verify_options([{connect_options, List} | Options]) when is_list(List) ->
+ verify_options(Options);
+verify_options([{proxy, List} | Options]) when is_list(List) ->
+ verify_options(Options);
+verify_options([{proxy_ssl_options, List} | Options]) when is_list(List) ->
+ verify_options(Options);
+verify_options([{pool, PidOrName} | Options])
when is_pid(PidOrName); is_atom(PidOrName) ->
- verify_options(Options, Errors);
-verify_options([Option | Options], Errors) ->
- verify_options(Options, [Option | Errors]);
-verify_options([], []) ->
- ok;
-verify_options([], Errors) ->
- bad_options(Errors).
-
--spec bad_options(options()) -> no_return().
-bad_options(Errors) ->
- erlang:error({bad_options, Errors}).
+ verify_options(Options);
+verify_options([Option | _Rest]) ->
+ erlang:error({bad_option, Option});
+verify_options([]) ->
+ ok.
--spec verify_partial_download(options(), options()) -> options().
-verify_partial_download([{window_size, infinity} | Options], Errors)->
- verify_partial_download(Options, Errors);
-verify_partial_download([{window_size, Size} | Options], Errors) when
+-spec verify_partial_download(options()) -> ok.
+verify_partial_download([{window_size, infinity} | Options])->
+ verify_partial_download(Options);
+verify_partial_download([{window_size, Size} | Options]) when
is_integer(Size), Size >= 0 ->
- verify_partial_download(Options, Errors);
-verify_partial_download([{part_size, Size} | Options], Errors) when
+ verify_partial_download(Options);
+verify_partial_download([{part_size, Size} | Options]) when
is_integer(Size), Size >= 0 ->
- verify_partial_download(Options, Errors);
-verify_partial_download([{part_size, infinity} | Options], Errors) ->
- verify_partial_download(Options, Errors);
-verify_partial_download([Option | Options], Errors) ->
- verify_partial_download(Options, [Option | Errors]);
-verify_partial_download([], Errors) ->
- Errors.
+ verify_partial_download(Options);
+verify_partial_download([{part_size, infinity} | Options]) ->
+ verify_partial_download(Options);
+verify_partial_download([Option | _Options]) ->
+ erlang:error({bad_option, {partial_download, Option}});
+verify_partial_download([]) ->
+ ok.
View
14 src/lhttpc_types.hrl
@@ -29,11 +29,23 @@
-type socket() :: _.
+-type invalid_option() :: any().
+
+-type partial_download_option() ::
+ {window_size, window_size()} |
+ {part_size, non_neg_integer() | infinity} |
+ invalid_option().
+
-type option() ::
{connect_timeout, timeout()} |
{send_retry, non_neg_integer()} |
{partial_upload, non_neg_integer() | infinity} |
- {partial_download, pid(), non_neg_integer() | infinity}.
+ {partial_download, [partial_download_option()]} |
+ {connect_options, socket_options()} |
+ {proxy, string()} |
+ {proxy_ssl_options, socket_options()} |
+ {pool, pid() | atom()} |
+ invalid_option().
-type options() :: [option()].
View
9 test/lhttpc_tests.erl
@@ -552,7 +552,7 @@ partial_upload_chunked_no_trailer() ->
lhttpc_lib:header_value("x-test-orig-body", headers(Response))).
partial_download_illegal_option() ->
- ?assertError({bad_options, [{partial_download, [{foo, bar}]}]},
+ ?assertError({bad_option, {partial_download, {foo, bar}}},
lhttpc:request("http://localhost/", get, [], <<>>, 1000,
[{partial_download, [{foo, bar}]}])).
@@ -729,9 +729,12 @@ connection_count() ->
?assertEqual(0, lhttpc_manager:connection_count(lhttpc_manager)).
invalid_options() ->
- ?assertError({bad_options, [{foo, bar}, bad_option]},
+ ?assertError({bad_option, bad_option},
lhttpc:request("http://localhost/", get, [], <<>>, 1000,
- [bad_option, {foo, bar}])).
+ [bad_option, {foo, bar}])),
+ ?assertError({bad_option, {foo, bar}},
+ lhttpc:request("http://localhost/", get, [], <<>>, 1000,
+ [{foo, bar}, bad_option])).
%%% Helpers functions
Something went wrong with that request. Please try again.