Skip to content

Commit d4fe96d

Browse files
committed
Request execution failure reason and redirects (#516)
* Introduces RequestExecutionErrorReason * Optionally track redirects in RequestExecutionError
1 parent 8f0b732 commit d4fe96d

File tree

7 files changed

+85
-19
lines changed

7 files changed

+85
-19
lines changed

wp_api/src/api_error.rs

+28-6
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ pub enum WpApiError {
1515
#[error("Status code ({}) is not valid", status_code)]
1616
InvalidHttpStatusCode { status_code: u16 },
1717
#[error(
18-
"Request execution failed!\nStatus Code: '{:?}'.\nResponse: '{}'",
18+
"Request execution failed!\nStatus Code: '{:?}'\nRedirects: '{:#?}'\nReason: '{:#?}'",
1919
status_code,
20+
redirects,
2021
reason
2122
)]
2223
RequestExecutionFailed {
2324
status_code: Option<u16>,
24-
reason: String,
25+
redirects: Option<Vec<String>>,
26+
reason: RequestExecutionErrorReason,
2527
},
2628
#[error("Media file not found at file path: {}", file_path)]
2729
MediaFileNotFound { file_path: String },
@@ -419,26 +421,42 @@ pub enum WpErrorCode {
419421
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
420422
pub enum RequestExecutionError {
421423
#[error(
422-
"Request execution failed!\nStatus Code: '{:?}'.\nResponse: '{}'",
424+
"Request execution failed!\nStatus Code: '{:?}'\nRedirects: '{:#?}'\nReason: '{:#?}'",
423425
status_code,
426+
redirects,
424427
reason
425428
)]
426429
RequestExecutionFailed {
427430
status_code: Option<u16>,
428-
reason: String,
431+
redirects: Option<Vec<String>>,
432+
reason: RequestExecutionErrorReason,
433+
},
434+
}
435+
436+
#[derive(Debug, Clone, PartialEq, Eq, uniffi::Enum)]
437+
pub enum RequestExecutionErrorReason {
438+
SslError {
439+
domain: String,
440+
trust_chain: Option<Vec<String>>,
441+
error_message: String,
442+
},
443+
GenericError {
444+
error_message: String,
429445
},
430446
}
431447

432448
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
433449
pub enum MediaUploadRequestExecutionError {
434450
#[error(
435-
"Request execution failed!\nStatus Code: '{:?}'.\nResponse: '{}'",
451+
"Request execution failed!\nStatus Code: '{:?}'\nRedirects: '{:#?}'\nReason: '{:#?}'",
436452
status_code,
453+
redirects,
437454
reason
438455
)]
439456
RequestExecutionFailed {
440457
status_code: Option<u16>,
441-
reason: String,
458+
redirects: Option<Vec<String>>,
459+
reason: RequestExecutionErrorReason,
442460
},
443461
#[error("Media file not found at file path: {}", file_path)]
444462
MediaFileNotFound { file_path: String },
@@ -449,9 +467,11 @@ impl From<RequestExecutionError> for WpApiError {
449467
match value {
450468
RequestExecutionError::RequestExecutionFailed {
451469
status_code,
470+
redirects,
452471
reason,
453472
} => Self::RequestExecutionFailed {
454473
status_code,
474+
redirects,
455475
reason,
456476
},
457477
}
@@ -463,9 +483,11 @@ impl From<MediaUploadRequestExecutionError> for WpApiError {
463483
match value {
464484
MediaUploadRequestExecutionError::RequestExecutionFailed {
465485
status_code,
486+
redirects,
466487
reason,
467488
} => Self::RequestExecutionFailed {
468489
status_code,
490+
redirects,
469491
reason,
470492
},
471493
MediaUploadRequestExecutionError::MediaFileNotFound { file_path } => {

wp_api/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
pub use api_client::{WpApiClient, WpApiRequestBuilder};
44
pub use api_error::{
5-
MediaUploadRequestExecutionError, ParsedRequestError, RequestExecutionError, WpApiError,
6-
WpError, WpErrorCode,
5+
MediaUploadRequestExecutionError, ParsedRequestError, RequestExecutionError,
6+
RequestExecutionErrorReason, WpApiError, WpError, WpErrorCode,
77
};
88
pub use parsed_url::{ParseUrlError, ParsedUrl};
99
use plugins::*;

wp_api/src/wordpress_org/client.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::{
2+
api_error::RequestExecutionErrorReason,
23
request::{endpoint::WpEndpointUrl, RequestExecutor, WpNetworkRequest, WpNetworkResponse},
34
wordpress_org::update_check::UpdateCheckRequest,
45
ParsedUrl, PluginWithViewContext, PluginWpOrgDirectorySlug, RequestExecutionError,
@@ -167,13 +168,15 @@ pub enum WordPressOrgApiClientError {
167168
#[error("Failed to encode request. Reason: {}", reason)]
168169
RequestEncodingError { reason: String },
169170
#[error(
170-
"Request execution failed!\nStatus Code: '{:?}'.\nResponse: '{}'",
171+
"Request execution failed!\nStatus Code: '{:?}'\nRedirects: '{:#?}'\nReason: '{:#?}'",
171172
status_code,
173+
redirects,
172174
reason
173175
)]
174176
RequestExecutionFailed {
175177
status_code: Option<u16>,
176-
reason: String,
178+
redirects: Option<Vec<String>>,
179+
reason: RequestExecutionErrorReason,
177180
},
178181
#[error("Error while parsing. \nReason: {}\nResponse: {}", reason, response)]
179182
ResponseParsingError { reason: String, response: String },
@@ -190,9 +193,11 @@ impl From<RequestExecutionError> for WordPressOrgApiClientError {
190193
match e {
191194
RequestExecutionError::RequestExecutionFailed {
192195
status_code,
196+
redirects,
193197
reason,
194198
} => WordPressOrgApiClientError::RequestExecutionFailed {
195199
status_code,
200+
redirects,
196201
reason,
197202
},
198203
}

wp_api_integration_tests/src/lib.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ use wp_api::{
1313
},
1414
tags::TagId,
1515
users::UserId,
16-
MediaUploadRequestExecutionError, ParsedUrl, RequestExecutionError, WpApiClient, WpApiError,
17-
WpAuthentication, WpErrorCode,
16+
MediaUploadRequestExecutionError, ParsedUrl, RequestExecutionError,
17+
RequestExecutionErrorReason, WpApiClient, WpApiError, WpAuthentication, WpErrorCode,
1818
};
1919

2020
// A `TestCredentials::instance()` function will be generated by this
@@ -154,7 +154,10 @@ pub struct AsyncWpNetworking {
154154
impl Default for AsyncWpNetworking {
155155
fn default() -> Self {
156156
Self {
157-
client: reqwest::Client::new(),
157+
client: reqwest::Client::builder()
158+
.danger_accept_invalid_certs(true)
159+
.build()
160+
.expect("We should be able to build the reqwest client with this configuration"),
158161
}
159162
}
160163
}
@@ -242,7 +245,10 @@ impl RequestExecutor for AsyncWpNetworking {
242245
self.async_request(request).await.map_err(|err| {
243246
RequestExecutionError::RequestExecutionFailed {
244247
status_code: err.status().map(|s| s.as_u16()),
245-
reason: err.to_string(),
248+
redirects: None,
249+
reason: RequestExecutionErrorReason::GenericError {
250+
error_message: err.to_string(),
251+
},
246252
}
247253
})
248254
}
@@ -256,7 +262,10 @@ impl RequestExecutor for AsyncWpNetworking {
256262
.map_err(
257263
|err| MediaUploadRequestExecutionError::RequestExecutionFailed {
258264
status_code: err.status().map(|s| s.as_u16()),
259-
reason: err.to_string(),
265+
redirects: None,
266+
reason: RequestExecutionErrorReason::GenericError {
267+
error_message: err.to_string(),
268+
},
260269
},
261270
)
262271
}

wp_api_integration_tests/tests/test_login_err.rs

+20
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,23 @@ async fn test_login_flow_err_parse_api_details(#[case] site_url: &str) {
2525
original_attempt_error
2626
);
2727
}
28+
29+
#[rstest]
30+
#[case("http://jalib923knblakis9ba92q3nbaslkes.nope")]
31+
#[tokio::test]
32+
#[parallel]
33+
async fn test_login_flow_err_network_error(#[case] site_url: &str) {
34+
let client = WpLoginClient::new(Arc::new(AsyncWpNetworking::default()));
35+
let mut result = client.api_discovery(site_url.to_string()).await;
36+
let original_attempt_error = result
37+
.attempts
38+
.remove(&AutoDiscoveryAttemptType::UserInput)
39+
.unwrap()
40+
.result
41+
.unwrap_err();
42+
assert!(
43+
original_attempt_error.is_network_error(),
44+
"{:#?}",
45+
original_attempt_error
46+
);
47+
}

wp_api_integration_tests/tests/test_login_immut.rs

+4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ const VANILLA_WP_SITE_URL: &str = "https://vanilla.wpmt.co/wp-admin/authorize-ap
5050
"https://jetpack.wpmt.co",
5151
"https://jetpack.wpmt.co/wp-admin/authorize-application.php"
5252
)]
53+
#[case(
54+
"http://wordpress-1315525-4803651.cloudwaysapps.com",
55+
"https://vanilla.wpmt.co/wp-admin/authorize-application.php"
56+
)]
5357
#[tokio::test]
5458
#[parallel]
5559
async fn test_login_flow(#[case] site_url: &str, #[case] expected_auth_url: &str) {

wp_api_integration_tests/tests/test_media_err.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use wp_api::{
1111
WpNetworkRequest, WpNetworkResponse,
1212
},
1313
users::UserId,
14-
MediaUploadRequestExecutionError, RequestExecutionError, WpApiClient, WpAuthentication,
15-
WpErrorCode,
14+
MediaUploadRequestExecutionError, RequestExecutionError, RequestExecutionErrorReason,
15+
WpApiClient, WpAuthentication, WpErrorCode,
1616
};
1717
use wp_api_integration_tests::{
1818
api_client, api_client_as_author, api_client_as_subscriber, test_site_url, AssertWpError,
@@ -208,7 +208,10 @@ impl RequestExecutor for MediaErrNetworking {
208208
) -> Result<WpNetworkResponse, RequestExecutionError> {
209209
Err(RequestExecutionError::RequestExecutionFailed {
210210
status_code: None,
211-
reason: "Execute function is not necessary for these tests".to_string(),
211+
redirects: None,
212+
reason: RequestExecutionErrorReason::GenericError {
213+
error_message: "Execute function is not necessary for these tests".to_string(),
214+
},
212215
})
213216
}
214217

@@ -242,7 +245,10 @@ impl RequestExecutor for MediaErrNetworking {
242245
let mut response = request.send().await.map_err(|err| {
243246
MediaUploadRequestExecutionError::RequestExecutionFailed {
244247
status_code: err.status().map(|s| s.as_u16()),
245-
reason: err.to_string(),
248+
redirects: None,
249+
reason: RequestExecutionErrorReason::GenericError {
250+
error_message: err.to_string(),
251+
},
246252
}
247253
})?;
248254

0 commit comments

Comments
 (0)