diff --git a/include/wx/private/webrequest.h b/include/wx/private/webrequest.h index a2f9299ca2..9a54668ae3 100644 --- a/include/wx/private/webrequest.h +++ b/include/wx/private/webrequest.h @@ -65,8 +65,6 @@ public: bool SetData(wxScopedPtr& dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset); - void SetIgnoreServerErrorStatus(bool ignore) { m_ignoreServerErrorStatus = ignore; } - void SetStorage(wxWebRequest::Storage storage) { m_storage = storage; } wxWebRequest::Storage GetStorage() const { return m_storage; } @@ -111,7 +109,9 @@ protected: wxWebRequestImpl(wxWebSession& session, wxEvtHandler* handler, int id); - bool CheckServerStatus(); + // Call SetState() with either State_Failed or State_Completed appropriate + // for the response status. + void SetFinalStateFromStatus(); static bool IsActiveState(wxWebRequest::State state); @@ -120,7 +120,6 @@ private: wxEvtHandler* const m_handler; const int m_id; wxWebRequest::State m_state; - bool m_ignoreServerErrorStatus; wxFileOffset m_bytesReceived; wxCharBuffer m_dataText; diff --git a/include/wx/webrequest.h b/include/wx/webrequest.h index 42e6312ad5..52499915c8 100644 --- a/include/wx/webrequest.h +++ b/include/wx/webrequest.h @@ -157,8 +157,6 @@ public: bool SetData(wxInputStream* dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset); - void SetIgnoreServerErrorStatus(bool ignore); - void SetStorage(Storage storage); Storage GetStorage() const; diff --git a/interface/wx/webrequest.h b/interface/wx/webrequest.h index 1d8cf06a36..11e592eebc 100644 --- a/interface/wx/webrequest.h +++ b/interface/wx/webrequest.h @@ -129,18 +129,33 @@ public: State_Idle, /** - The request is currently unauthorized. Calling GetAuthChallenge() - returns a challenge object with further details. + The request is currently unauthorized. + + Calling GetAuthChallenge() returns a challenge object with further + details and calling SetCredentials() on this object will retry the + request using these credentials. */ State_Unauthorized, /// The request has been started and is transferring data State_Active, - /// The request completed successfully and all data has been received. + /** + The request completed successfully and all data has been received. + + The HTTP status code returned by wxWebResponse::GetStatus() will be + in 100-399 range, and typically 200. + */ State_Completed, - /// The request failed + /** + The request failed. + + This can happen either because the request couldn't be performed at + all (e.g. a connection error) or if the server returned an HTTP + error. In the former case wxWebResponse::GetStatus() returns 0, + while in the latter it returns a value in 400-599 range. + */ State_Failed, /// The request has been cancelled before completion by calling Cancel() @@ -321,18 +336,6 @@ public: bool SetData(wxInputStream* dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset); - /** - Instructs the request to ignore server error status codes. - - Per default, server side errors (status code 400-599) will enter - the State_Failed state just like network errors, but - if the response is still required in such cases (e.g. to get more - details from the response body), set this option to ignore all errors. - If ignored, wxWebResponse::GetStatus() has to be checked - from the State_Completed event handler. - */ - void SetIgnoreServerErrorStatus(bool ignore); - /** Sets how response data will be stored. diff --git a/src/common/webrequest.cpp b/src/common/webrequest.cpp index 93e967a896..a405b3fcef 100644 --- a/src/common/webrequest.cpp +++ b/src/common/webrequest.cpp @@ -63,22 +63,28 @@ wxWebRequestImpl::wxWebRequestImpl(wxWebSession& session, wxEvtHandler* handler, m_handler(handler), m_id(id), m_state(wxWebRequest::State_Idle), - m_ignoreServerErrorStatus(false), m_bytesReceived(0) { } -bool wxWebRequestImpl::CheckServerStatus() +void wxWebRequestImpl::SetFinalStateFromStatus() { const wxWebResponseImplPtr& resp = GetResponse(); - if ( resp && resp->GetStatus() >= 400 && !m_ignoreServerErrorStatus ) + if ( !resp || resp->GetStatus() >= 400 ) { - SetState(wxWebRequest::State_Failed, wxString::Format(_("Error: %s (%d)"), - resp->GetStatusText(), resp->GetStatus())); - return false; + wxString err; + if ( resp ) + { + err.Printf(_("Error: %s (%d)"), + resp->GetStatusText(), resp->GetStatus()); + } + + SetState(wxWebRequest::State_Failed, err); } else - return true; + { + SetState(wxWebRequest::State_Completed); + } } bool wxWebRequestImpl::IsActiveState(wxWebRequest::State state) @@ -360,13 +366,6 @@ wxWebRequest::SetData(wxInputStream* dataStream, return m_impl->SetData(streamPtr, contentType, dataSize); } -void wxWebRequest::SetIgnoreServerErrorStatus(bool ignore) -{ - wxCHECK_IMPL_VOID(); - - m_impl->SetIgnoreServerErrorStatus(ignore); -} - void wxWebRequest::SetStorage(Storage storage) { wxCHECK_IMPL_VOID(); diff --git a/src/common/webrequest_curl.cpp b/src/common/webrequest_curl.cpp index 3e9c7053cd..d2927039b9 100644 --- a/src/common/webrequest_curl.cpp +++ b/src/common/webrequest_curl.cpp @@ -272,16 +272,20 @@ void wxWebRequestCURL::HandleCompletion() { int status = m_response ? m_response->GetStatus() : 0; - if ( status == 0) + if ( status == 0 ) + { SetState(wxWebRequest::State_Failed, GetError()); + } else if ( status == 401 || status == 407 ) { m_authChallenge.reset(new wxWebAuthChallengeCURL( (status == 407) ? wxWebAuthChallenge::Source_Proxy : wxWebAuthChallenge::Source_Server, *this)); SetState(wxWebRequest::State_Unauthorized, m_response->GetStatusText()); } - else if ( CheckServerStatus() ) - SetState(wxWebRequest::State_Completed); + else + { + SetFinalStateFromStatus(); + } } wxString wxWebRequestCURL::GetError() const diff --git a/src/msw/webrequest_winhttp.cpp b/src/msw/webrequest_winhttp.cpp index 9aa07a0f3d..99ba8380d0 100644 --- a/src/msw/webrequest_winhttp.cpp +++ b/src/msw/webrequest_winhttp.cpp @@ -170,7 +170,7 @@ wxWebRequestWinHTTP::HandleCallback(DWORD dwInternetStatus, } else { - SetState(wxWebRequest::State_Completed); + SetFinalStateFromStatus(); } break; @@ -259,7 +259,7 @@ void wxWebRequestWinHTTP::CreateResponse() else SetFailedWithLastError(); } - else if ( CheckServerStatus() ) + else { // Start reading the response if ( !m_response->ReadData() ) diff --git a/src/osx/webrequest_urlsession.mm b/src/osx/webrequest_urlsession.mm index fb8a126458..822c65dda9 100644 --- a/src/osx/webrequest_urlsession.mm +++ b/src/osx/webrequest_urlsession.mm @@ -235,8 +235,7 @@ void wxWebRequestURLSession::HandleCompletion() break; default: - if ( CheckServerStatus() ) - SetState(wxWebRequest::State_Completed); + SetFinalStateFromStatus(); } } diff --git a/tests/net/webrequest.cpp b/tests/net/webrequest.cpp index 115ae97b14..2b63851e12 100644 --- a/tests/net/webrequest.cpp +++ b/tests/net/webrequest.cpp @@ -192,6 +192,21 @@ TEST_CASE_METHOD(RequestFixture, Run(wxWebRequest::State_Failed, 404); } +TEST_CASE_METHOD(RequestFixture, + "WebRequest::Error::Body", "[net][webrequest][error]") +{ + if ( !InitBaseURL() ) + return; + + // We can't use the same httpbin.org server that we use for the other tests + // for this one because it doesn't return anything in the body when + // returning an error status code, so use another one. + CreateAbs("https://httpstat.us/418"); + Run(wxWebRequest::State_Failed, 418); + + CHECK( request.GetResponse().AsString() == "418 I'm a teapot" ); +} + TEST_CASE_METHOD(RequestFixture, "WebRequest::Error::Connect", "[net][webrequest][error]") {