Skip to content
This repository was archived by the owner on Dec 13, 2023. It is now read-only.

node-machine-id can cause crashes #34

Closed
Mike-Bell opened this issue Jul 6, 2021 · 4 comments
Closed

node-machine-id can cause crashes #34

Mike-Bell opened this issue Jul 6, 2021 · 4 comments

Comments

@Mike-Bell
Copy link

Mike-Bell commented Jul 6, 2021

as per this issue, node-machine-id can cause problems on certain machines due to permissions issues: automation-stack/node-machine-id#56

This was made worse when this codebase changed from fetching the machine id during the report generation to fetching the machine id statically: 48e3bdc#diff-19c5769b32e486be5464e2b259d16fd1217d53501e90ea7ed95b4dce09e7a40dR13

This essentially means that importing this library will cause the consuming app to immediately crash for some users

@sbahra
Copy link

sbahra commented Jul 6, 2021

Thank you for reporting this! We will investigate and work on a fix.

@Mike-Bell
Copy link
Author

This is pretty easy to repro if you disable access to your registry. I used method 2 here https://www.top-password.com/blog/3-ways-to-disable-registry-editor-in-windows/

rashkov pushed a commit to rashkov/backtrace-node that referenced this issue Jul 8, 2021
@sbahra
Copy link

sbahra commented Jul 9, 2021

We'll have a patch out next week, we have a fix in a personal branch for testing.

This was referenced Jul 12, 2021
konraddysput added a commit that referenced this issue Jul 14, 2021
* Vendor machine ID lib to fix issue #34

* Fix up win32 machineId() case

* Fix expose() for win32 case

* Fallback to generating ID when unable to get one

* Use hostname as a fall-back when machine id is undefined (#36)

* Use hostname as fall-back when machine id is undefined

* Use random bytes when hostname is undefined

* Version bump

Co-authored-by: Mike Rashkovsky <[email protected]>
Co-authored-by: Konrad Dysput <[email protected]>
@konraddysput
Copy link
Collaborator

Hey @sbahra @Mike-Bell

The new mechanism that we added to the [email protected] package should fix this issue. Thanks for sharing a lot of details with us @Mike-Bell. Please let us know if the newest version of the library doesn't help you.

Pull request reference: #37

Thanks!

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

No branches or pull requests

3 participants