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

classify errors which happen because a file is not pollable/cancellable #23

Open
mvdan opened this issue Oct 19, 2024 · 0 comments
Open

Comments

@mvdan
Copy link

mvdan commented Oct 19, 2024

When debugging a downstream issue (mvdan/sh#1099) I ended up figuring out that the cause was an EPERM error coming from creating an epoll when the provided file to NewReader is a regular file on Linux.

This is because epoll does not make sense on regular files, as they are always ready for read syscalls, so they are not supported by epoll: https://stackoverflow.com/questions/5603642/epoll-ctl-operation-not-permitted-error-c-program

I'm currently working around this issue by not using a cancellable reader if https://pkg.go.dev/github.com/muesli/cancelreader#NewReader returns a non-nil error. Which is fine, but it's a fairly nasty solution; for all I know, I could be using non-cancellable readers for all other sorts of reasons.

It would be nice if I could do something like errors.Is(err, cancelreader.ErrNotSupported) so that I could skip the cancel reader in those cases, but treat any other non-nil error as fatal. This would map to each of the "unsupported file" edge cases on each platform, such as EPERM errors on creating the epoll on Linux due to e.g. regular files.

Would that be an acceptable change? If so, then I think newFallbackCancelReader should also go away entirely - rather than doing a return newFallbackCancelReader(reader) on unsupported platforms or files, you would return nil, ErrNotSupported and let the caller decide what to do - such as using the original reader as-is without cancellation. This would be a minor breaking change, but a good one - if stability is important to you, then I would suggest adding a new API alongside NewReader with this behavior, for example TryReader, akin to the naming of https://pkg.go.dev/sync#Mutex.TryLock.

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