You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by xu...@apache.org on 2026/10/21 09:59:49 UTC
(opendal) branch main updated: fix(services): mark 5xx responses temporary in alluxio, pcloud and seafile (#8112)
This is an automated email from the ASF dual-hosted git repository.
Xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 188557781 fix(services): mark 5xx responses temporary in alluxio, pcloud and seafile (#8112)
188557781 is described below
commit 188557781ab478a9f3c0477ba47029a0e5d8abc6
Author: ZIHAN DAI <99...@users.noreply.github.com>
AuthorDate: Wed Oct 21 20:59:43 2026 +1100
fix(services): mark 5xx responses temporary in alluxio, pcloud and seafile (#8112)
A 5xx from these three is a transient backend failure and retrying is the
documented recovery, but parse_error returned it without set_temporary, so
the retry layer treated it as permanent and gave up on the first attempt.
alluxio's match already had a 500 arm that returned the same kind as the
catch-all, and seafile already carried a retryable flag that nothing read.
Rebuilt on current main: #8196 moved parse_error out of the inline error
module and gave it an ErrorContext, so the original diff no longer applied.
---
core/services/alluxio/src/core.rs | 37 ++++++++++++++++++++++++++++++++++---
core/services/pcloud/src/core.rs | 8 ++++++++
core/services/seafile/src/core.rs | 10 ++++++++--
3 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/core/services/alluxio/src/core.rs b/core/services/alluxio/src/core.rs
index 288911203..5a59bb262 100644
--- a/core/services/alluxio/src/core.rs
+++ b/core/services/alluxio/src/core.rs
@@ -404,6 +404,31 @@ mod tests {
assert_eq!(err.kind(), ErrorKind::Unsupported);
}
+ #[test]
+ fn test_parse_error_marks_5xx_temporary() {
+ for status in [500u16, 502, 503, 504] {
+ let resp = Response::builder()
+ .status(StatusCode::from_u16(status).unwrap())
+ .body(Buffer::from(bytes::Bytes::from_static(
+ br#"{"statusCode":"INTERNAL_SERVER_ERROR","message":"boom"}"#,
+ )))
+ .unwrap();
+
+ let err = parse_error(ErrorContext::new(ServiceOperation("Test")), resp);
+
+ assert!(err.is_temporary(), "{status} should be retryable");
+ }
+
+ let resp = Response::builder()
+ .status(StatusCode::BAD_REQUEST)
+ .body(Buffer::from(bytes::Bytes::from_static(
+ br#"{"statusCode":"INVALID_ARGUMENT","message":"bad"}"#,
+ )))
+ .unwrap();
+ let err = parse_error(ErrorContext::new(ServiceOperation("Test")), resp);
+ assert!(!err.is_temporary(), "400 must not be retryable");
+ }
+
/// Error response example is from https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html
#[test]
fn test_parse_error() {
@@ -508,9 +533,11 @@ pub(crate) fn parse_error(ctx: ErrorContext, resp: Response<Buffer>) -> Error {
let (parts, body) = resp.into_parts();
let bs = body.to_bytes();
- let mut kind = match parts.status.as_u16() {
- 500 => ErrorKind::Unexpected,
- _ => ErrorKind::Unexpected,
+ let (mut kind, retryable) = match parts.status.as_u16() {
+ // Alluxio reports a transient master or worker failure as a 5xx; retrying is the
+ // documented recovery, so mark it temporary rather than surfacing it as permanent.
+ 500 | 502 | 503 | 504 => (ErrorKind::Unexpected, true),
+ _ => (ErrorKind::Unexpected, false),
};
let (message, alluxio_err) = serde_json::from_reader::<_, AlluxioError>(bs.clone().reader())
@@ -530,5 +557,9 @@ pub(crate) fn parse_error(ctx: ErrorContext, resp: Response<Buffer>) -> Error {
err = err.with_context("service_operation", ctx.service_operation.0);
err = with_error_response_context(err, parts);
+ if retryable {
+ err = err.set_temporary();
+ }
+
err
}
diff --git a/core/services/pcloud/src/core.rs b/core/services/pcloud/src/core.rs
index 738f6e42e..b50786bdb 100644
--- a/core/services/pcloud/src/core.rs
+++ b/core/services/pcloud/src/core.rs
@@ -563,11 +563,19 @@ pub(crate) fn parse_error(ctx: ErrorContext, resp: Response<Buffer>) -> Error {
let bs = body.to_bytes();
let message = String::from_utf8_lossy(&bs).into_owned();
+ // pCloud returns a 5xx for a transient backend failure; retrying is the documented
+ // recovery, so mark it temporary rather than surfacing it as permanent.
+ let retryable = matches!(parts.status.as_u16(), 500 | 502 | 503 | 504);
+
let mut err = Error::new(ErrorKind::Unexpected, message);
err = err.with_context("service_operation", ctx.service_operation.0);
err = with_error_response_context(err, parts);
+ if retryable {
+ err = err.set_temporary();
+ }
+
err
}
diff --git a/core/services/seafile/src/core.rs b/core/services/seafile/src/core.rs
index beceabe01..ffff40784 100644
--- a/core/services/seafile/src/core.rs
+++ b/core/services/seafile/src/core.rs
@@ -561,10 +561,12 @@ pub(crate) fn parse_error(ctx: ErrorContext, resp: Response<Buffer>) -> Error {
let (parts, body) = resp.into_parts();
let bs = body.to_bytes();
- let (kind, _retryable) = match parts.status.as_u16() {
+ let (kind, retryable) = match parts.status.as_u16() {
403 => (ErrorKind::PermissionDenied, false),
404 => (ErrorKind::NotFound, false),
- 520 => (ErrorKind::Unexpected, false),
+ // Seafile reports a transient backend failure as a 5xx, including its own 520;
+ // retrying is the documented recovery, so mark these temporary.
+ 500 | 502 | 503 | 504 | 520 => (ErrorKind::Unexpected, true),
_ => (ErrorKind::Unexpected, false),
};
@@ -577,6 +579,10 @@ pub(crate) fn parse_error(ctx: ErrorContext, resp: Response<Buffer>) -> Error {
err = err.with_context("service_operation", ctx.service_operation.0);
err = with_error_response_context(err, parts);
+ if retryable {
+ err = err.set_temporary();
+ }
+
err
}