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

fix: reject on non-zero exit codes #2046

Merged
merged 2 commits into from
May 7, 2020
Merged

Conversation

Swaagie
Copy link
Contributor

@Swaagie Swaagie commented Mar 5, 2020

udevadm does not expose errors when exporting the database (yet). But it does exit with code > 0, if no errors are exposed the exit code can still be used to provide feedback even if generic rather than just returning an empty array.

Happy to change the thrown error message if it does not match requirements.

Background: systemd/systemd#14959 wrong permissions on any device that udevadm attempts to list will throw EACCES. These errors are not shown but strace will show an exit code 1

Copy link
Member

@reconbot reconbot left a comment

Choose a reason for hiding this comment

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

This is a really great catch I had no idea we weren’t handling errors here.

packages/bindings/lib/linux-list.js Outdated Show resolved Hide resolved
@Swaagie
Copy link
Contributor Author

Swaagie commented Mar 7, 2020

Updated, wasn't obvious at all that udevadm failed. I think on most systems it just always works. I just happened to have a pi3 that was being an outlier (read: I failed to set it up correctly 😄)

@stale stale bot removed the stale-issue label May 7, 2020
@serialport serialport deleted a comment from stale bot May 7, 2020
@reconbot reconbot merged commit 6ee5c84 into serialport:master May 7, 2020
@Swaagie Swaagie deleted the exit-code branch May 11, 2020 21:41
lvogt pushed a commit to lvogt/node-serialport that referenced this pull request Aug 10, 2021
* fix: reject on non-zero exit codes
* fix: use more descriptive error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants