Skip to content

Commit dd2e68f

Browse files
authored
refactor(services/sftp): Impl parse_error instead of From<Error> (#3914)
1 parent 45270f8 commit dd2e68f

3 files changed

Lines changed: 48 additions & 66 deletions

File tree

core/src/services/sftp/backend.rs

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ use serde::Deserialize;
3434

3535
use super::error::is_not_found;
3636
use super::error::is_sftp_protocol_error;
37+
use super::error::parse_sftp_error;
38+
use super::error::parse_ssh_error;
3739
use super::lister::SftpLister;
3840
use super::writer::SftpWriter;
3941
use crate::raw::*;
@@ -291,7 +293,7 @@ impl Accessor for SftpBackend {
291293
if let Err(e) = res {
292294
// ignore error if dir already exists
293295
if !is_sftp_protocol_error(&e) {
294-
return Err(e.into());
296+
return Err(parse_sftp_error(e));
295297
}
296298
}
297299
fs.set_cwd(&current);
@@ -305,7 +307,7 @@ impl Accessor for SftpBackend {
305307
let mut fs = client.fs();
306308
fs.set_cwd(&self.root);
307309

308-
let meta: Metadata = fs.metadata(path).await?.into();
310+
let meta: Metadata = fs.metadata(path).await.map_err(parse_sftp_error)?.into();
309311

310312
Ok(RpStat::new(meta))
311313
}
@@ -315,9 +317,12 @@ impl Accessor for SftpBackend {
315317

316318
let mut fs = client.fs();
317319
fs.set_cwd(&self.root);
318-
let path = fs.canonicalize(path).await?;
320+
let path = fs.canonicalize(path).await.map_err(parse_sftp_error)?;
319321

320-
let f = client.open(path.as_path()).await?;
322+
let f = client
323+
.open(path.as_path())
324+
.await
325+
.map_err(parse_sftp_error)?;
321326

322327
// Sorry for the ugly code...
323328
//
@@ -339,7 +344,7 @@ impl Accessor for SftpBackend {
339344

340345
let mut fs = client.fs();
341346
fs.set_cwd(&self.root);
342-
let path = fs.canonicalize(path).await?;
347+
let path = fs.canonicalize(path).await.map_err(parse_sftp_error)?;
343348

344349
let mut option = client.options();
345350
option.create(true);
@@ -349,7 +354,7 @@ impl Accessor for SftpBackend {
349354
option.write(true);
350355
}
351356

352-
let file = option.open(path).await?;
357+
let file = option.open(path).await.map_err(parse_sftp_error)?;
353358

354359
Ok((RpWrite::new(), SftpWriter::new(file)))
355360
}
@@ -368,15 +373,15 @@ impl Accessor for SftpBackend {
368373
if is_not_found(&e) {
369374
return Ok(RpDelete::default());
370375
} else {
371-
return Err(e.into());
376+
return Err(parse_sftp_error(e));
372377
}
373378
}
374379
}
375380
.read_dir()
376381
.boxed();
377382

378383
while let Some(file) = dir.next().await {
379-
let file = file?;
384+
let file = file.map_err(parse_sftp_error)?;
380385
let file_name = file.filename().to_str();
381386
if file_name == Some(".") || file_name == Some("..") {
382387
continue;
@@ -394,14 +399,14 @@ impl Accessor for SftpBackend {
394399

395400
match fs.remove_dir(path).await {
396401
Err(e) if !is_not_found(&e) => {
397-
return Err(e.into());
402+
return Err(parse_sftp_error(e));
398403
}
399404
_ => {}
400405
}
401406
} else {
402407
match fs.remove_file(path).await {
403408
Err(e) if !is_not_found(&e) => {
404-
return Err(e.into());
409+
return Err(parse_sftp_error(e));
405410
}
406411
_ => {}
407412
}
@@ -423,7 +428,7 @@ impl Accessor for SftpBackend {
423428
if is_not_found(&e) {
424429
return Ok((RpList::default(), None));
425430
} else {
426-
return Err(e.into());
431+
return Err(parse_sftp_error(e));
427432
}
428433
}
429434
}
@@ -445,12 +450,15 @@ impl Accessor for SftpBackend {
445450
self.create_dir(dir, OpCreateDir::default()).await?;
446451
}
447452

448-
let src = fs.canonicalize(from).await?;
449-
let dst = fs.canonicalize(to).await?;
450-
let mut src_file = client.open(&src).await?;
451-
let mut dst_file = client.create(dst).await?;
453+
let src = fs.canonicalize(from).await.map_err(parse_sftp_error)?;
454+
let dst = fs.canonicalize(to).await.map_err(parse_sftp_error)?;
455+
let mut src_file = client.open(&src).await.map_err(parse_sftp_error)?;
456+
let mut dst_file = client.create(dst).await.map_err(parse_sftp_error)?;
452457

453-
src_file.copy_all_to(&mut dst_file).await?;
458+
src_file
459+
.copy_all_to(&mut dst_file)
460+
.await
461+
.map_err(parse_sftp_error)?;
454462

455463
Ok(RpCopy::default())
456464
}
@@ -464,7 +472,7 @@ impl Accessor for SftpBackend {
464472
if let Some((dir, _)) = to.rsplit_once('/') {
465473
self.create_dir(dir, OpCreateDir::default()).await?;
466474
}
467-
fs.rename(from, to).await?;
475+
fs.rename(from, to).await.map_err(parse_sftp_error)?;
468476

469477
Ok(RpRename::default())
470478
}
@@ -517,9 +525,11 @@ async fn connect_sftp(
517525

518526
session.known_hosts_check(known_hosts_strategy);
519527

520-
let session = session.connect(&endpoint).await?;
528+
let session = session.connect(&endpoint).await.map_err(parse_ssh_error)?;
521529

522-
let sftp = Sftp::from_session(session, SftpOptions::default()).await?;
530+
let sftp = Sftp::from_session(session, SftpOptions::default())
531+
.await
532+
.map_err(parse_sftp_error)?;
523533

524534
if !root.is_empty() {
525535
let mut fs = sftp.fs();
@@ -533,7 +543,7 @@ async fn connect_sftp(
533543
if let Err(e) = res {
534544
// ignore error if dir already exists
535545
if !is_sftp_protocol_error(&e) {
536-
return Err(e.into());
546+
return Err(parse_sftp_error(e));
537547
}
538548
}
539549
fs.set_cwd(&current);

core/src/services/sftp/error.rs

Lines changed: 13 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -22,55 +22,23 @@ use openssh_sftp_client::Error as SftpClientError;
2222
use crate::Error;
2323
use crate::ErrorKind;
2424

25-
#[derive(Debug)]
26-
pub enum SftpError {
27-
SftpClientError(SftpClientError),
28-
SshError(SshError),
29-
}
30-
31-
impl From<SftpClientError> for Error {
32-
fn from(e: SftpClientError) -> Self {
33-
let kind = match &e {
34-
SftpClientError::UnsupportedSftpProtocol { version: _ } => ErrorKind::Unsupported,
35-
SftpClientError::SftpError(kind, _msg) => match kind {
36-
SftpErrorKind::NoSuchFile => ErrorKind::NotFound,
37-
SftpErrorKind::PermDenied => ErrorKind::PermissionDenied,
38-
SftpErrorKind::OpUnsupported => ErrorKind::Unsupported,
39-
_ => ErrorKind::Unexpected,
40-
},
25+
pub fn parse_sftp_error(e: SftpClientError) -> Error {
26+
let kind = match &e {
27+
SftpClientError::UnsupportedSftpProtocol { version: _ } => ErrorKind::Unsupported,
28+
SftpClientError::SftpError(kind, _msg) => match kind {
29+
SftpErrorKind::NoSuchFile => ErrorKind::NotFound,
30+
SftpErrorKind::PermDenied => ErrorKind::PermissionDenied,
31+
SftpErrorKind::OpUnsupported => ErrorKind::Unsupported,
4132
_ => ErrorKind::Unexpected,
42-
};
43-
44-
Error::new(kind, "sftp error").set_source(e)
45-
}
46-
}
47-
48-
/// REMOVE ME: it's not allowed to impl `<T>` for Error.
49-
impl From<SshError> for Error {
50-
fn from(e: SshError) -> Self {
51-
Error::new(ErrorKind::Unexpected, "ssh error").set_source(e)
52-
}
53-
}
54-
55-
impl From<SftpClientError> for SftpError {
56-
fn from(e: SftpClientError) -> Self {
57-
SftpError::SftpClientError(e)
58-
}
59-
}
33+
},
34+
_ => ErrorKind::Unexpected,
35+
};
6036

61-
impl From<SshError> for SftpError {
62-
fn from(e: SshError) -> Self {
63-
SftpError::SshError(e)
64-
}
37+
Error::new(kind, "sftp error").set_source(e)
6538
}
6639

67-
impl From<SftpError> for Error {
68-
fn from(e: SftpError) -> Self {
69-
match e {
70-
SftpError::SftpClientError(e) => e.into(),
71-
SftpError::SshError(e) => e.into(),
72-
}
73-
}
40+
pub fn parse_ssh_error(e: SshError) -> Error {
41+
Error::new(ErrorKind::Unexpected, "ssh error").set_source(e)
7442
}
7543

7644
pub(super) fn is_not_found(e: &SftpClientError) -> bool {

core/src/services/sftp/lister.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use openssh_sftp_client::fs::ReadDir;
2828
use crate::raw::oio;
2929
use crate::Result;
3030

31+
use super::error::parse_sftp_error;
32+
3133
pub struct SftpLister {
3234
dir: Pin<Box<ReadDir>>,
3335
prefix: String,
@@ -47,7 +49,9 @@ impl SftpLister {
4749
#[async_trait]
4850
impl oio::List for SftpLister {
4951
fn poll_next(&mut self, cx: &mut Context<'_>) -> Poll<Result<Option<oio::Entry>>> {
50-
let item = ready!(self.dir.poll_next_unpin(cx)).transpose()?;
52+
let item = ready!(self.dir.poll_next_unpin(cx))
53+
.transpose()
54+
.map_err(parse_sftp_error)?;
5155

5256
match item {
5357
Some(e) => {

0 commit comments

Comments
 (0)