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

server: use os.IsNotExist to map sshFxNoSuchFile #382

Merged
merged 2 commits into from
Sep 15, 2020
Merged

server: use os.IsNotExist to map sshFxNoSuchFile #382

merged 2 commits into from
Sep 15, 2020

Conversation

willnorris
Copy link
Contributor

Always use os.IsNotExist to identify any OS specific error types that represent missing files or directories. This resolves an issue on Windows where some system errors (ENOTDIR) were not being identified as 'not found' errors and mapped to sshFxNoSuchFile.

fixes #381

Always use os.IsNotExist to identify any OS specific error types that
represent missing files or directories.  This resolves an issue on
Windows where some system errors (ENOTDIR) were not being identified as
'not found' errors and mapped to sshFxNoSuchFile.

fixes #381
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 14, 2020
update github.com/bradfitz/latlong in order to pick up
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp for pkg/sftp#382
which resolves CI test failures for Windows.
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 14, 2020
update github.com/bradfitz/latlong in order to pick up
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp for pkg/sftp#382
which resolves CI test failures for Windows.
Copy link
Collaborator

@puellanivis puellanivis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to refactor to cover all os.IsNotExist(…) cases better.

At the very least, we should be using os.IsNotExist(e) as well, instead of the more fragile case os.ErrNotExist:

Return early from statusFromError if os.IsNotExist is true.
@willnorris
Copy link
Contributor Author

technically, this means we could also remove the syscall.ENOENT case from inside of translateError, since that is only ever called from statusFromError. I think it could still be reasonable to leave it there, but happy to remove it if you'd like.

@puellanivis
Copy link
Collaborator

Up to you, if you want to remove the ENOENT case. Like you say, we should never hit it anymore. But I am going to guess that it’s not actually too much code or binary bloat, so it might be better to keep it around as a just in case backup? 🤷‍♀️

I’m fine either way.

@willnorris
Copy link
Contributor Author

willnorris commented Sep 14, 2020

yeah, I think probably leave it in. 👍 Given what the function is doing... mapping syscall error numbers to ssh error codes, it makes sense for it to have fuller coverage of different error numbers. That keeps it somewhat semantically independent of the statusFromError func.

@puellanivis puellanivis merged commit 37434b9 into pkg:master Sep 15, 2020
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 15, 2020
update github.com/bradfitz/latlong in order to include
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp to include
pkg/sftp#382 which resolves CI test failures for
Windows.
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 15, 2020
update github.com/bradfitz/latlong in order to include
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp to include
pkg/sftp#382 which resolves CI test failures for
Windows.
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 23, 2020
update github.com/bradfitz/latlong in order to include
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp to include
pkg/sftp#382 which resolves CI test failures for
Windows.
willnorris added a commit to willnorris/perkeep that referenced this pull request Sep 24, 2020
update github.com/bradfitz/latlong in order to include
bradfitz/latlong@afb97c1f9 which updates the
import path of freetype-go.  The old path no longer exists and was
causing problems with go mod.

update github.com/pkg/sftp to include
pkg/sftp#382 which resolves CI test failures for
Windows.
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

Successfully merging this pull request may close these issues.

server Stat call returns incorrect error for some non-existent Windows file paths
2 participants