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

node: Return traverse error as os.PathError #134

Merged
merged 1 commit into from
Dec 10, 2017
Merged

node: Return traverse error as os.PathError #134

merged 1 commit into from
Dec 10, 2017

Conversation

imsodin
Copy link
Contributor

@imsodin imsodin commented Nov 20, 2017

Missing nodes are already reported as os.PathError. This does the same for the error returned when setting up a recursive watch with a non-recursive watcher, i.e. during traversing. The error string is the same as before, so user of this library wont notice the change. This allows to check the underlying error and which path causes the problem. An example of a use case is for Syncthing, where we want to notify the user when the error is due to hitting the inotify handler limit.

@imsodin
Copy link
Contributor Author

imsodin commented Dec 10, 2017

@ppknap @rjeczalik While you're dealing with the other PR, could you also have a look at this (then we can update our vendoring all in one ;) ).

Copy link
Collaborator

@ppknap ppknap left a comment

Choose a reason for hiding this comment

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

@imsodin I don't know why we haven't merged this yet. This change is perfectly fine :)

@rjeczalik rjeczalik merged commit db28320 into rjeczalik:master Dec 10, 2017
@rjeczalik
Copy link
Owner

I must have missed notification on this one, thanks @imsodin!

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.

3 participants