Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return meaningful error when subsystem request for sftp failed #133

Closed
fd0 opened this issue Aug 28, 2016 · 2 comments
Closed

Return meaningful error when subsystem request for sftp failed #133

fd0 opened this issue Aug 28, 2016 · 2 comments

Comments

@fd0
Copy link
Contributor

fd0 commented Aug 28, 2016

Hi,

I'm writing a backup program (restic) which uses the sftp library. Recently, a user reported an issue with using his NAS with restic over sftp.

It turned out that sftp was deactivated for the NAS, but restic only printed a generic error message:

subsystem request failed on channel 0

I'd like to be able to catch this error and replace it with a more meaningful message for my users.

Unfortunately, the original error (from ssh/session) is created like this:

err = errors.New("ssh: subsystem request failed")

and the sftp library just passes it up the stack (here:

if err := s.RequestSubsystem("sftp"); err != nil {
    return nil, err
}

So in order to distinguish this error from others, the only way is to match against the string. That's ugly.

Would you accept a PR which returns a custom wrapper error type when RequestSubsystem returns an error? Something along the lines:

type subsystemRequestFailed string

func (err subsystemRequestFailed) Error() string {
   return string(err)
}

func (err subsystemRequestFailed) SubsystemRequestFailed() bool {
   return true
}

[...]
// NewClient creates a new SFTP client on conn, using zero or more option
// functions.
func NewClient(conn *ssh.Client, opts ...func(*Client) error) (*Client, error) {
    s, err := conn.NewSession()
    if err != nil {
        return nil, err
    }
    if err := s.RequestSubsystem("sftp"); err != nil {
        return nil, subsystemRequestFailed(err.Error())
    }
    pw, err := s.StdinPipe()
    if err != nil {
        return nil, err
    }
    pr, err := s.StdoutPipe()
    if err != nil {
        return nil, err
    }

    return NewClientPipe(pr, pw, opts...)
}

[...]

Then the caller can test the behavior of the error by checking if it implements the method SubsystemRequestFailed().

Does that sound good? Any better idea for the method name?

@fd0
Copy link
Contributor Author

fd0 commented Aug 28, 2016

Hm, digging deeper into this I recognize that the error which showed up for the top-level code is not created in RequestSubsystem, the error string is slightly different: subsystem request failed on channel 0 vs ssh: subsystem request failed

Since I can't find the string anywhere in the Go code, I suspect that the error is printed by my ssh client program. I therefore close this issue, because this has nothing to do with the sftp package.

I'm sorry for everybody who read my journey here ;)

@fd0 fd0 closed this as completed Aug 28, 2016
@fd0
Copy link
Contributor Author

fd0 commented Aug 28, 2016

Solution to this issue: The error message is indeed printed by the ssh program I started. The error is printed for server which do not offer the sftp subsystem. Mystery solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant