aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMicael Karlberg <[email protected]>2010-02-12 15:55:58 +0000
committerErlang/OTP <[email protected]>2010-02-13 07:29:10 +0100
commitd9286a9a68e8b10dcf006a60ec84a0439e725fac (patch)
treed49cd8f54778f192593be6943b1661b9d1772d7e
parentba3758531a0b6dc6797457279e1ce0f99d282d20 (diff)
downloadotp-d9286a9a68e8b10dcf006a60ec84a0439e725fac.tar.gz
otp-d9286a9a68e8b10dcf006a60ec84a0439e725fac.tar.bz2
otp-d9286a9a68e8b10dcf006a60ec84a0439e725fac.zip
OTP-8431: Fix error handling of httpc_manager and its starter process
-rw-r--r--lib/inets/doc/src/httpc.xml4
-rw-r--r--lib/inets/doc/src/notes.xml4
-rw-r--r--lib/inets/src/http_client/httpc_handler.erl11
-rw-r--r--lib/inets/src/http_client/httpc_manager.erl112
-rw-r--r--lib/inets/test/httpc_SUITE.erl22
-rw-r--r--lib/inets/vsn.mk2
6 files changed, 121 insertions, 34 deletions
diff --git a/lib/inets/doc/src/httpc.xml b/lib/inets/doc/src/httpc.xml
index e143ba2c1a..ca93190f61 100644
--- a/lib/inets/doc/src/httpc.xml
+++ b/lib/inets/doc/src/httpc.xml
@@ -277,7 +277,7 @@ ssl_options() = {verify, code()} |
stream_start, Headers}, {http, {RequestId, stream,
BinBodyPart}, {http, {RequestId, stream_end, Headers}. When
streaming to to the calling processes using the option
- <c>{self once}</c> the first message will have an additional
+ <c>{self, once}</c> the first message will have an additional
element e.i. {http, {RequestId, stream_start, Headers, Pid},
this is the process id that should be used as an argument to
http:stream_next/1 to trigger the next message to be sent to
@@ -329,7 +329,7 @@ ssl_options() = {verify, code()} |
<p>Note that the validity of the options are <em>not</em>
checked in any way. </p>
<p>Note that this may change the socket behaviour
- (see <seealso marker="inet#setopts">inet:setopts/2</seealso>)
+ (see <seealso marker="kernel:inet#setopts">inet:setopts/2</seealso>)
for an already existing, and therefor already connected
request handler. </p>
<p>By defaults the socket options set by the
diff --git a/lib/inets/doc/src/notes.xml b/lib/inets/doc/src/notes.xml
index e95c8d6e97..0c524f00d1 100644
--- a/lib/inets/doc/src/notes.xml
+++ b/lib/inets/doc/src/notes.xml
@@ -354,7 +354,7 @@
request, when the client connects to the server. Default
value is that of the <c>timeout</c> option. </p>
<p>See the
- <seealso marker="http#request2">request/4,5</seealso>
+ <seealso marker="httpc#request2">request/4,5</seealso>
function for more info. </p>
<p>Own Id: OTP-7298</p>
<!-- <p>Aux Id: seq11086</p> -->
@@ -461,7 +461,7 @@
the client connects to the server. </p>
<p>As a side-effect of this, the option <c>ipv6</c> has been
removed and replaced by the <c>ipfamily</c> option. </p>
- <p>See <seealso marker="http#set_options">http:set_options/1,2</seealso>
+ <p>See <seealso marker="httpc#set_options">http:set_options/1,2</seealso>
for more info. </p>
<p>*** POTENTIAL INCOMPATIBILITY ***</p>
<p>Own Id: OTP-8004</p>
diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index fec74932a2..31585537d4 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -267,9 +267,14 @@ handle_call({connect_and_send, #request{address = Address0,
%%send_ssl_tunnel_request(Address, Request,
%% #state{options = Options,
%% status = ssl_tunnel});
- Reason = https_through_proxy_is_not_currently_supported,
- Error = {error, Reason},
- {stop, Error, Error, State};
+ Reason = {failed_connecting,
+ https_through_proxy_is_not_currently_supported},
+ %% Send a reply to the original caller
+ ErrorResponse = httpc_response:error(Request, Reason),
+ httpc_response:send(Request#request.from, ErrorResponse),
+ %% Reply to the manager
+ ErrorReply = {error, Reason},
+ {stop, normal, ErrorReply, State};
true ->
case connect_and_send_first_request(Address, Request, State) of
{ok, NewState} ->
diff --git a/lib/inets/src/http_client/httpc_manager.erl b/lib/inets/src/http_client/httpc_manager.erl
index f8fc6322ed..f3cd81f4a7 100644
--- a/lib/inets/src/http_client/httpc_manager.erl
+++ b/lib/inets/src/http_client/httpc_manager.erl
@@ -62,7 +62,7 @@
starter, % Pid of the handler starter process (temp): pid()
handler, % Pid of the handler process: pid()
from, % From for the request: from()
- state % State of the handler: initiating | operational | canceled
+ state % State of the handler: initiating | started | operational | canceled
}).
%% Entries in the handler / request cross-ref table
@@ -539,6 +539,11 @@ handle_cast(Msg, #state{profile_name = ProfileName} = State) ->
%% {stop, Reason, State} (terminate/2 is called)
%% Description: Handling all non call/cast messages
%%---------------------------------------------------------
+
+handle_info({started, StarterPid, ReqId, HandlerPid}, State) ->
+ handle_started(StarterPid, ReqId, HandlerPid, State),
+ {noreply, State};
+
handle_info({connect_and_send, StarterPid, ReqId, HandlerPid, Res}, State) ->
handle_connect_and_send(StarterPid, ReqId, HandlerPid, Res, State),
{noreply, State};
@@ -633,6 +638,38 @@ get_handler_info(Tab) ->
%%
%% The request handler process is started asynchronously by a
+%% "starter process". When the handler has sucessfully been started,
+%% this message (started) is sent.
+%%
+
+handle_started(StarterPid, ReqId, HandlerPid,
+ #state{profile_name = Profile,
+ handler_db = HandlerDb}) ->
+ case ets:lookup(HandlerDb, ReqId) of
+ [#handler_info{state = initiating} = HandlerInfo] ->
+ ?hcri("received started ack for initiating handler", []),
+ HandlerInfo2 = HandlerInfo#handler_info{handler = HandlerPid,
+ state = started},
+ ets:insert(HandlerDb, HandlerInfo2),
+ ok;
+
+ [#handler_info{state = State}] ->
+ error_report(Profile,
+ "unexpected (started) message for handler (~p) in state "
+ "~p regarding request ~p - ignoring", [HandlerPid, State, ReqId]),
+ ?hcri("received unexpected started message", [{state, State}]),
+ ok;
+
+ [] ->
+ error_report(Profile,
+ "unknown handler ~p (~p) started for request ~w - canceling",
+ [HandlerPid, StarterPid, ReqId]),
+ httpc_handler:cancel(ReqId, HandlerPid)
+ end.
+
+
+%%
+%% The request handler process is started asynchronously by a
%% "starter process". When that process terminates it sends
%% one of two messages. These ara handled by the two functions
%% below.
@@ -642,8 +679,8 @@ handle_connect_and_send(_StarterPid, ReqId, HandlerPid, Result,
#state{profile_name = Profile,
handler_db = HandlerDb}) ->
case ets:lookup(HandlerDb, ReqId) of
- [#handler_info{state = initiating} = HandlerInfo] when Result =:= ok ->
- ?hcri("received connect-and-send ack for initiating handler", []),
+ [#handler_info{state = started} = HandlerInfo] when Result =:= ok ->
+ ?hcri("received connect-and-send ack for started handler", []),
HandlerInfo2 = HandlerInfo#handler_info{starter = undefined,
handler = HandlerPid,
state = operational},
@@ -658,28 +695,24 @@ handle_connect_and_send(_StarterPid, ReqId, HandlerPid, Result,
ets:insert(HandlerDb, HandlerInfo2),
ok;
- [#handler_info{from = From}] ->
+ [#handler_info{state = State}] when Result =/= ok ->
error_report(Profile,
- "handler (~p) failed to connect and/or "
+ "handler (~p, ~w) failed to connect and/or "
"send request ~p"
- "~n Error: ~p", [HandlerPid, ReqId, Result]),
- ?hcri("received connect-and-send error", [{result, Result}]),
- Reason2 =
- case Result of
- {error, Reason} ->
- {failed_connecting, Reason};
- _ ->
- {failed_connecting, Result}
- end,
- DummyReq = #request{id = ReqId},
- httpc_response:send(From, httpc_response:error(DummyReq, Reason2)),
- %% gen_server:reply(From, Error),
+ "~n Result: ~p",
+ [HandlerPid, State, ReqId, Result]),
+ ?hcri("received connect-and-send error",
+ [{result, Result}, {state, State}]),
+ %% We don't need to send a response to the original caller
+ %% because the handler already sent one in its terminate
+ %% function.
ets:delete(HandlerDb, ReqId),
ok;
[] ->
error_report(Profile,
- "handler successfully (~p) started for unknown request ~p",
+ "handler (~p) successfully started "
+ "for unknown request ~p => canceling",
[HandlerPid, ReqId]),
httpc_handler:cancel(ReqId, HandlerPid)
end.
@@ -689,13 +722,11 @@ handle_failed_starting_handler(_StarterPid, ReqId, Error,
#state{profile_name = Profile,
handler_db = HandlerDb}) ->
case ets:lookup(HandlerDb, ReqId) of
- [#handler_info{state = canceled,
- from = From}] ->
+ [#handler_info{state = canceled}] ->
error_report(Profile,
"failed starting handler for request ~p"
"~n Error: ~p", [ReqId, Error]),
request_canceled(Profile, ReqId), % Fake signal from handler
- gen_server:reply(From, Error),
ets:delete(HandlerDb, ReqId),
ok;
@@ -703,7 +734,15 @@ handle_failed_starting_handler(_StarterPid, ReqId, Error,
error_report(Profile,
"failed starting handler for request ~p"
"~n Error: ~p", [ReqId, Error]),
- gen_server:reply(From, Error),
+ Reason2 =
+ case Error of
+ {error, Reason} ->
+ {failed_connecting, Reason};
+ _ ->
+ {failed_connecting, Error}
+ end,
+ DummyReq = #request{id = ReqId},
+ httpc_response:send(From, httpc_response:error(DummyReq, Reason2)),
ets:delete(HandlerDb, ReqId),
ok;
@@ -719,10 +758,32 @@ maybe_handle_terminating_starter(MeybeStarterPid, Reason, HandlerDb) ->
Pattern = #handler_info{starter = MeybeStarterPid, _ = '_'},
case ets:match_object(HandlerDb, Pattern) of
[#handler_info{id = ReqId, from = From, state = initiating}] ->
- Error = {error, {failed_starting_request_handler, Reason}},
- gen_server:reply(From, Error),
+ %% The starter process crashed before it could start the
+ %% the handler process, therefor we need to answer the
+ %% original caller.
+ ?hcri("starter process crashed bfore starting handler",
+ [{starter, MeybeStarterPid}, {reason, Reason}]),
+ Reason2 =
+ case Reason of
+ {error, Error} ->
+ {failed_connecting, Error};
+ _ ->
+ {failed_connecting, Reason}
+ end,
+ DummyReq = #request{id = ReqId},
+ httpc_response:send(From, httpc_response:error(DummyReq, Reason2)),
ets:delete(HandlerDb, ReqId),
ok;
+
+ [#handler_info{state = State} = HandlerInfo] ->
+ %% The starter process crashed after the handler was started.
+ %% The handler will answer to the original caller.
+ ?hcri("starter process crashed after starting handler",
+ [{starter, MeybeStarterPid}, {reason, Reason}, {state, State}]),
+ HandlerInfo2 = HandlerInfo#handler_info{starter = undefined},
+ ets:insert(HandlerDb, HandlerInfo2),
+ ok;
+
_ ->
ok
end.
@@ -886,6 +947,9 @@ create_handler_starter(#request{id = Id,
[{id, Id}, {profile, ProfileName}, {result, Result1}]),
case Result1 of
{ok, HandlerPid} ->
+ StartedMessage =
+ {started, self(), Id, HandlerPid},
+ ManagerPid ! StartedMessage,
Result2 = httpc_handler:connect_and_send(Request,
HandlerPid),
?hcrd("handler starter - connected and sent",
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 96099c49fd..aa65fb1197 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -1440,13 +1440,31 @@ proxy_page_does_not_exist(Config) when is_list(Config) ->
%%-------------------------------------------------------------------------
+
proxy_https_not_supported(doc) ->
[];
proxy_https_not_supported(suite) ->
[];
proxy_https_not_supported(Config) when is_list(Config) ->
- {error, {failed_connecting, https_through_proxy_is_not_currently_supported}} =
- http:request(get, {"https://login.yahoo.com", []}, [], []),
+ Result = http:request(get, {"https://login.yahoo.com", []}, [], []),
+ case Result of
+ {error, Reason} ->
+ %% ok so far
+ case Reason of
+ {failed_connecting, Why} ->
+ %% ok, now check why
+ case Why of
+ https_through_proxy_is_not_currently_supported ->
+ ok;
+ _ ->
+ tsf({unexpected_why, Why})
+ end;
+ _ ->
+ tsf({unexpected_reason, Reason})
+ end;
+ _ ->
+ tsf({unexpected_result, Result})
+ end,
ok.
diff --git a/lib/inets/vsn.mk b/lib/inets/vsn.mk
index 746517eed5..c905532066 100644
--- a/lib/inets/vsn.mk
+++ b/lib/inets/vsn.mk
@@ -19,7 +19,7 @@
APPLICATION = inets
INETS_VSN = 5.3
-PRE_VSN =-p13
+PRE_VSN =-p14
APP_VSN = "$(APPLICATION)-$(INETS_VSN)$(PRE_VSN)"
TICKETS = \