Skip to content

Commit 6c03f2c

Browse files
muirdmmeta-codesync[bot]
muirdm
authored and
meta-codesync[bot]
committed
edenapi: shrink large remote API error variants
Summary: Box the large `SaplingRemoteApiError` payloads instead of returning the whole large payload inline. This keeps `Result<T, SaplingRemoteApiError>` unchanged while addressing the `clippy::result_large_err` lint for the remote API error type. Reviewed By: zzl0 Differential Revision: D102548170 fbshipit-source-id: 4871419d731af4028f901b02c4c5e567b42942ab
1 parent f8f7c83 commit 6c03f2c

4 files changed

Lines changed: 35 additions & 31 deletions

File tree

‎eden/scm/lib/backingstore/src/ffi_errors.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ mod tests {
157157
SaplingRemoteApiError::HttpError {
158158
status: http::StatusCode::NOT_FOUND,
159159
message: "Not found".to_string(),
160-
headers: http::HeaderMap::new(),
160+
headers: Box::new(http::HeaderMap::new()),
161161
url: "https://example.com".to_string(),
162162
},
163163
BackingStoreErrorKind::Network,

‎eden/scm/lib/eagerepo/src/api.rs‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ impl SaplingRemoteApi for EagerRepo {
201201
Err(SaplingRemoteApiError::HttpError {
202202
status: StatusCode::INTERNAL_SERVER_ERROR,
203203
message: "failpoint".to_string(),
204-
headers: Default::default(),
204+
headers: Box::default(),
205205
url: self.url("files_attrs"),
206206
})
207207
});
@@ -319,7 +319,7 @@ impl SaplingRemoteApi for EagerRepo {
319319
Err(SaplingRemoteApiError::HttpError {
320320
status: StatusCode::INTERNAL_SERVER_ERROR,
321321
message: "failpoint".to_string(),
322-
headers: Default::default(),
322+
headers: Box::default(),
323323
url: self.url("trees"),
324324
})
325325
});
@@ -726,7 +726,7 @@ impl SaplingRemoteApi for EagerRepo {
726726
return Err(SaplingRemoteApiError::HttpError {
727727
status: StatusCode::BAD_REQUEST,
728728
message: "must specify one of 'to' or 'from'".to_string(),
729-
headers: Default::default(),
729+
headers: Box::default(),
730730
url: self.url("set_bookmark"),
731731
});
732732
}
@@ -737,7 +737,7 @@ impl SaplingRemoteApi for EagerRepo {
737737
return Err(SaplingRemoteApiError::HttpError {
738738
status: StatusCode::NOT_FOUND,
739739
message: format!("bookmark {bookmark} doesn't exist"),
740-
headers: Default::default(),
740+
headers: Box::default(),
741741
url: self.url("set_bookmark"),
742742
});
743743
}
@@ -748,7 +748,7 @@ impl SaplingRemoteApi for EagerRepo {
748748
message: format!(
749749
"bookmark {bookmark}'s current value is {node}, not {from}"
750750
),
751-
headers: Default::default(),
751+
headers: Box::default(),
752752
url: self.url("set_bookmark"),
753753
});
754754
}
@@ -758,7 +758,7 @@ impl SaplingRemoteApi for EagerRepo {
758758
return Err(SaplingRemoteApiError::HttpError {
759759
status: StatusCode::BAD_REQUEST,
760760
message: format!("bookmark {bookmark} already exists"),
761-
headers: Default::default(),
761+
headers: Box::default(),
762762
url: self.url("set_bookmark"),
763763
});
764764
}
@@ -1001,7 +1001,7 @@ impl SaplingRemoteApi for EagerRepo {
10011001
Some("error") => Err(SaplingRemoteApiError::HttpError {
10021002
status: StatusCode::INTERNAL_SERVER_ERROR,
10031003
message: "failpoint".to_string(),
1004-
headers: Default::default(),
1004+
headers: Box::default(),
10051005
url: self.url("upload_changesets"),
10061006
}),
10071007
Some("empty") => Ok(convert_to_response(Vec::new())),
@@ -1071,7 +1071,7 @@ impl SaplingRemoteApi for EagerRepo {
10711071
return Err(SaplingRemoteApiError::HttpError {
10721072
status: StatusCode::INTERNAL_SERVER_ERROR,
10731073
message: format!("error inserting mutation entry: {:?}", err),
1074-
headers: Default::default(),
1074+
headers: Box::default(),
10751075
url: self.url("upload_changesets"),
10761076
});
10771077
}
@@ -1129,7 +1129,7 @@ impl SaplingRemoteApi for EagerRepo {
11291129
return Err(SaplingRemoteApiError::HttpError {
11301130
status: StatusCode::INTERNAL_SERVER_ERROR,
11311131
message: format!("{:?}", e),
1132-
headers: Default::default(),
1132+
headers: Box::default(),
11331133
url: self.url("lookup_batch"),
11341134
});
11351135
}
@@ -1159,7 +1159,7 @@ impl SaplingRemoteApi for EagerRepo {
11591159
return Err(SaplingRemoteApiError::HttpError {
11601160
status: StatusCode::BAD_REQUEST,
11611161
message: "from_repo and to_repo not supported".to_string(),
1162-
headers: Default::default(),
1162+
headers: Box::default(),
11631163
url: self.url("commit_translate_id"),
11641164
});
11651165
}
@@ -1168,7 +1168,7 @@ impl SaplingRemoteApi for EagerRepo {
11681168
return Err(SaplingRemoteApiError::HttpError {
11691169
status: StatusCode::BAD_REQUEST,
11701170
message: "only hg and bonsai supported".to_string(),
1171-
headers: Default::default(),
1171+
headers: Box::default(),
11721172
url: self.url("commit_translate_id"),
11731173
});
11741174
}
@@ -1198,7 +1198,7 @@ impl SaplingRemoteApi for EagerRepo {
11981198
return Err(SaplingRemoteApiError::HttpError {
11991199
status: StatusCode::BAD_REQUEST,
12001200
message: "only hg and bonsai supported".to_string(),
1201-
headers: Default::default(),
1201+
headers: Box::default(),
12021202
url: self.url("commit_translate_id"),
12031203
});
12041204
}
@@ -1328,7 +1328,7 @@ impl SaplingRemoteApi for EagerRepo {
13281328
return Err(SaplingRemoteApiError::HttpError {
13291329
status: StatusCode::NOT_FOUND,
13301330
message: format!("bookmark {} was not found", bookmark),
1331-
headers: Default::default(),
1331+
headers: Box::default(),
13321332
url: self.url("land_stack"),
13331333
});
13341334
}
@@ -1640,7 +1640,7 @@ impl EagerRepo {
16401640
None => Err(SaplingRemoteApiError::HttpError {
16411641
status: StatusCode::NOT_FOUND,
16421642
message: format!("{} cannot be found", id.to_hex()),
1643-
headers: Default::default(),
1643+
headers: Box::default(),
16441644
url: self.url(handler),
16451645
}),
16461646
Some(data) => Ok(data),
@@ -1665,7 +1665,7 @@ impl EagerRepo {
16651665
Err(e) => Err(SaplingRemoteApiError::HttpError {
16661666
status: StatusCode::INTERNAL_SERVER_ERROR,
16671667
message: format!("{:?}", e),
1668-
headers: Default::default(),
1668+
headers: Box::default(),
16691669
url: self.url(handler),
16701670
}),
16711671
}
@@ -1683,7 +1683,7 @@ impl EagerRepo {
16831683
return Err(SaplingRemoteApiError::HttpError {
16841684
status: StatusCode::INTERNAL_SERVER_ERROR,
16851685
message: format!("{:?}", e),
1686-
headers: Default::default(),
1686+
headers: Box::default(),
16871687
url: self.url(handler),
16881688
});
16891689
}
@@ -1692,7 +1692,7 @@ impl EagerRepo {
16921692
return Err(SaplingRemoteApiError::HttpError {
16931693
status: StatusCode::BAD_REQUEST,
16941694
message: "content hash mismatch".to_string(),
1695-
headers: Default::default(),
1695+
headers: Box::default(),
16961696
url: self.url(handler),
16971697
});
16981698
}
@@ -1718,7 +1718,7 @@ impl EagerRepo {
17181718
Err(e) => Err(SaplingRemoteApiError::HttpError {
17191719
status: StatusCode::INTERNAL_SERVER_ERROR,
17201720
message: format!("{:?}", e),
1721-
headers: Default::default(),
1721+
headers: Box::default(),
17221722
url: self.url(handler),
17231723
}),
17241724
}
@@ -1734,7 +1734,7 @@ impl EagerRepo {
17341734
None => Err(SaplingRemoteApiError::HttpError {
17351735
status: StatusCode::NOT_FOUND,
17361736
message: format!("{} cannot be found", id.to_hex()),
1737-
headers: Default::default(),
1737+
headers: Box::default(),
17381738
url: self.url(handler),
17391739
}),
17401740
Some(data) => Ok(data),
@@ -1747,7 +1747,7 @@ impl EagerRepo {
17471747
.map_err(|err| SaplingRemoteApiError::HttpError {
17481748
status: StatusCode::INTERNAL_SERVER_ERROR,
17491749
message: format!("error flushing dag/store: {:?}", err),
1750-
headers: Default::default(),
1750+
headers: Box::default(),
17511751
url: self.url(handler),
17521752
})
17531753
}
@@ -1757,7 +1757,7 @@ impl EagerRepo {
17571757
SaplingRemoteApiError::HttpError {
17581758
status: StatusCode::NOT_IMPLEMENTED,
17591759
message,
1760-
headers: Default::default(),
1760+
headers: Box::default(),
17611761
url: self.url(handler),
17621762
}
17631763
}

‎eden/scm/lib/edenapi/src/client.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,9 +1637,8 @@ impl SaplingRemoteApi for Client {
16371637
let flattened_response = response
16381638
.into_iter()
16391639
.map(|res| {
1640-
res.data.map_err(|err| {
1641-
SaplingRemoteApiError::ServerError(SaplingRemoteApiServerError::new(err))
1642-
})
1640+
res.data
1641+
.map_err(|err| SaplingRemoteApiServerError::new(err).into())
16431642
})
16441643
.collect::<Result<Vec<BookmarkEntry>, _>>();
16451644
return flattened_response;
@@ -1669,9 +1668,8 @@ impl SaplingRemoteApi for Client {
16691668
response
16701669
.into_iter()
16711670
.map(|res| {
1672-
res.data.map_err(|err| {
1673-
SaplingRemoteApiError::ServerError(SaplingRemoteApiServerError::new(err))
1674-
})
1671+
res.data
1672+
.map_err(|err| SaplingRemoteApiServerError::new(err).into())
16751673
})
16761674
.collect::<Result<Vec<BookmarkEntry>, _>>()
16771675
}
@@ -2328,7 +2326,7 @@ async fn raise_for_status(res: AsyncResponse) -> Result<AsyncResponse, SaplingRe
23282326
Err(SaplingRemoteApiError::HttpError {
23292327
status,
23302328
message,
2331-
headers,
2329+
headers: Box::new(headers),
23322330
url,
23332331
})
23342332
}

‎eden/scm/lib/edenapi/trait/src/errors.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@ pub enum SaplingRemoteApiError {
3131
HttpError {
3232
status: StatusCode,
3333
message: String,
34-
headers: HeaderMap,
34+
headers: Box<HeaderMap>,
3535
url: String,
3636
},
3737
#[error(transparent)]
3838
InvalidUrl(#[from] url::ParseError),
3939
#[error(transparent)]
4040
WireToApiConversionFailed(#[from] WireToApiConversionError),
4141
#[error(transparent)]
42-
ServerError(#[from] SaplingRemoteApiServerError),
42+
ServerError(Box<SaplingRemoteApiServerError>),
4343
#[error("expected response, but none returned by the server")]
4444
NoResponse,
4545
#[error(transparent)]
@@ -62,6 +62,12 @@ pub enum ConfigError {
6262
Invalid(String, #[source] anyhow::Error),
6363
}
6464

65+
impl From<SaplingRemoteApiServerError> for SaplingRemoteApiError {
66+
fn from(err: SaplingRemoteApiServerError) -> Self {
67+
Self::ServerError(Box::new(err))
68+
}
69+
}
70+
6571
impl SaplingRemoteApiError {
6672
pub fn is_rate_limiting(&self) -> bool {
6773
use SaplingRemoteApiError::*;

0 commit comments

Comments
 (0)