From 6dc1c0d2b0da6693857bafca96a1619a1a0b698e Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Mon, 23 Jan 2012 15:31:34 +0100 Subject: [PATCH] Removed most of the "retry" in rescue clauses, since it was hidding critical issue. This should be the choice of the application to "retry" on exception, otherwise let the framework handle this in the lower part. Better handling of response termination (alias commit) Added the notion of "status_committed" --- .../request/rest/src/rest_request_handler.e | 27 ++++++++---------- .../connectors/nino/src/wgi_nino_connector.e | 28 ++++--------------- .../specification/response/wgi_response.e | 12 ++++++-- .../ewsgi/src/helper/wgi_response_stream.e | 8 ++++-- library/server/ewsgi/src/wgi_service.e | 4 +-- library/server/wsf/router/routed_service_i.e | 19 ++----------- library/server/wsf/src/wsf_response.e | 8 +++++- library/server/wsf/src/wsf_service.e | 11 ++++++-- 8 files changed, 54 insertions(+), 63 deletions(-) diff --git a/draft/library/server/request/rest/src/rest_request_handler.e b/draft/library/server/request/rest/src/rest_request_handler.e index df202468..8f0ed70e 100644 --- a/draft/library/server/request/rest/src/rest_request_handler.e +++ b/draft/library/server/request/rest/src/rest_request_handler.e @@ -34,27 +34,20 @@ feature -- Execution execute (ctx: C; req: WSF_REQUEST; res: WSF_RESPONSE) -- Execute request handler - local - rescued: BOOLEAN do - if not rescued then - if request_method_name_supported (req.request_method) then - if authentication_required (req) and then not authenticated (ctx) then - execute_unauthorized (ctx, req, res) - else - pre_execute (ctx, req, res) - execute_application (ctx, req, res) - post_execute (ctx, req, res) - end + if request_method_name_supported (req.request_method) then + if authentication_required (req) and then not authenticated (ctx) then + execute_unauthorized (ctx, req, res) else - execute_request_method_not_allowed (req, res, supported_request_method_names) + pre_execute (ctx, req, res) + execute_application (ctx, req, res) + post_execute (ctx, req, res) end else - rescue_execute (ctx, req, res) + execute_request_method_not_allowed (req, res, supported_request_method_names) end rescue - rescued := True - retry + execute_rescue (ctx, req, res) end execute_application (ctx: C; req: WSF_REQUEST; res: WSF_RESPONSE) @@ -69,9 +62,11 @@ feature -- Execution do end - rescue_execute (ctx: C; req: WSF_REQUEST; res: WSF_RESPONSE) + execute_rescue (ctx: C; req: WSF_REQUEST; res: WSF_RESPONSE) do post_execute (ctx, req, res) + rescue + --| Just in case, the rescue is raising other exceptions ... end execute_unauthorized (ctx: C; req: WSF_REQUEST; res: WSF_RESPONSE) diff --git a/library/server/ewsgi/connectors/nino/src/wgi_nino_connector.e b/library/server/ewsgi/connectors/nino/src/wgi_nino_connector.e index dc111270..72a51e7d 100644 --- a/library/server/ewsgi/connectors/nino/src/wgi_nino_connector.e +++ b/library/server/ewsgi/connectors/nino/src/wgi_nino_connector.e @@ -118,32 +118,16 @@ feature -- Server local req: WGI_REQUEST_FROM_TABLE res: detachable WGI_RESPONSE_STREAM - rescued: INTEGER do - if rescued = 0 then - create req.make (env, create {WGI_NINO_INPUT_STREAM}.make (a_socket), Current) - create res.make (create {WGI_NINO_OUTPUT_STREAM}.make (a_socket)) - req.set_meta_string_variable ("RAW_HEADER_DATA", a_headers_text) - service.execute (req, res) - elseif rescued = 1 then - if attached (create {EXCEPTION_MANAGER}).last_exception as e and then attached e.exception_trace as l_trace then - if res /= Void then - if not res.status_is_set then - res.set_status_code ({HTTP_STATUS_CODE}.internal_server_error) - end - if res.message_writable then - res.put_string ("
" + l_trace + "
") - end - end - end - end - rescue - rescued := rescued + 1 - retry + create req.make (env, create {WGI_NINO_INPUT_STREAM}.make (a_socket), Current) + create res.make (create {WGI_NINO_OUTPUT_STREAM}.make (a_socket)) + req.set_meta_string_variable ("RAW_HEADER_DATA", a_headers_text) + service.execute (req, res) + res.commit end note - copyright: "2011-2011, Eiffel Software and others" + copyright: "2011-2012, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/server/ewsgi/specification/response/wgi_response.e b/library/server/ewsgi/specification/response/wgi_response.e index 5ef7611d..e24829d1 100644 --- a/library/server/ewsgi/specification/response/wgi_response.e +++ b/library/server/ewsgi/specification/response/wgi_response.e @@ -6,7 +6,7 @@ note deferred class WGI_RESPONSE -feature {WGI_RESPONSE, WGI_SERVICE} -- Commit +feature {WGI_CONNECTOR, WGI_SERVICE} -- Commit commit -- Commit the current response @@ -49,6 +49,14 @@ feature -- Status setting deferred end + status_committed: BOOLEAN + -- Is status code set and committed? + -- i.e: sent to the client and could not be changed anymore + deferred + ensure + committed_implies_set: Result implies status_is_set + end + set_status_code (a_code: INTEGER) -- Set response status code -- Should be done before sending any data back to the client @@ -116,7 +124,7 @@ feature -- Output operation end note - copyright: "2011-2011, Eiffel Software and others" + copyright: "2011-2012, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/server/ewsgi/src/helper/wgi_response_stream.e b/library/server/ewsgi/src/helper/wgi_response_stream.e index 43f9bd10..d2a2cee2 100644 --- a/library/server/ewsgi/src/helper/wgi_response_stream.e +++ b/library/server/ewsgi/src/helper/wgi_response_stream.e @@ -22,7 +22,7 @@ feature {NONE} -- Initialization output := a_output end -feature {WGI_SERVICE} -- Commit +feature {WGI_CONNECTOR, WGI_SERVICE} -- Commit commit -- Commit the current response @@ -33,6 +33,9 @@ feature {WGI_SERVICE} -- Commit feature -- Status report + status_committed: BOOLEAN + -- Status code set and committed? + header_committed: BOOLEAN -- Header committed? @@ -68,6 +71,7 @@ feature -- Status setting do status_code := a_code output.put_status_line (a_code) + status_committed := True end status_code: INTEGER @@ -129,7 +133,7 @@ feature {NONE} -- Implementation: Access -- Server output channel ;note - copyright: "2011-2011, Eiffel Software and others" + copyright: "2011-2012, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/server/ewsgi/src/wgi_service.e b/library/server/ewsgi/src/wgi_service.e index a71cdb48..fe81383d 100644 --- a/library/server/ewsgi/src/wgi_service.e +++ b/library/server/ewsgi/src/wgi_service.e @@ -11,7 +11,7 @@ note deferred class WGI_SERVICE -feature -- Execution +feature {WGI_CONNECTOR} -- Execution execute (req: WGI_REQUEST; res: WGI_RESPONSE) -- Execute the request @@ -26,7 +26,7 @@ feature -- Execution end note - copyright: "2011-2011, Eiffel Software and others" + copyright: "2011-2012, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/server/wsf/router/routed_service_i.e b/library/server/wsf/router/routed_service_i.e index 30f80e97..55daeaf7 100644 --- a/library/server/wsf/router/routed_service_i.e +++ b/library/server/wsf/router/routed_service_i.e @@ -38,15 +38,10 @@ feature -- Execution execute (req: WSF_REQUEST; res: WSF_RESPONSE) local l_handled: BOOLEAN - rescued: BOOLEAN do - if not rescued then - l_handled := router.dispatch (req, res) - if not l_handled then - execute_default (req, res) - end - else - execute_rescue (req, res) + l_handled := router.dispatch (req, res) + if not l_handled then + execute_default (req, res) end end @@ -54,14 +49,6 @@ feature -- Execution deferred end - execute_rescue (req: WSF_REQUEST; res: WSF_RESPONSE) - do - if not res.header_committed then - res.put_header ({HTTP_STATUS_CODE}.internal_server_error, Void) - end - res.flush - end - note copyright: "2011-2011, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" diff --git a/library/server/wsf/src/wsf_response.e b/library/server/wsf/src/wsf_response.e index e6a1bc6e..d9e5925f 100644 --- a/library/server/wsf/src/wsf_response.e +++ b/library/server/wsf/src/wsf_response.e @@ -26,6 +26,12 @@ feature {NONE} -- Initialization feature -- Status report + status_committed: BOOLEAN + -- Status line committed? + do + Result := wgi_response.status_committed + end + header_committed: BOOLEAN -- Header committed? do @@ -56,7 +62,7 @@ feature -- Status setting -- Set response status code -- Should be done before sending any data back to the client require - status_not_set: not status_is_set + status_not_set: not status_committed header_not_committed: not header_committed do wgi_response.set_status_code (a_code) diff --git a/library/server/wsf/src/wsf_service.e b/library/server/wsf/src/wsf_service.e index cb1a1733..c2b36e0e 100644 --- a/library/server/wsf/src/wsf_service.e +++ b/library/server/wsf/src/wsf_service.e @@ -26,11 +26,18 @@ feature -- Execution deferred end -feature -- WGI Execution +feature {WGI_CONNECTOR} -- WGI Execution wgi_execute (req: WGI_REQUEST; res: WGI_RESPONSE) + local + w_res: detachable WSF_RESPONSE do - execute (create {WSF_REQUEST}.make_from_wgi (req), create {WSF_RESPONSE}.make_from_wgi (res)) + create w_res.make_from_wgi (res) + execute (create {WSF_REQUEST}.make_from_wgi (req), w_res) + rescue + if w_res /= Void then + w_res.flush + end end end