Skip to content
This repository has been archived by the owner on Jan 4, 2020. It is now read-only.

Do not let update-notifier errors crash the bin #132

Merged
merged 4 commits into from
Apr 28, 2016

Conversation

nexdrew
Copy link
Contributor

@nexdrew nexdrew commented Apr 27, 2016

Found problem testing 3.7.0 with update-notifier file permissions. This is somewhat of an ugly hack fix to handle cases of running the npme bin between sudo/root and non-root users.

  1. Silently ignore errors (fs permission problems against ~/.config/configstore/update-notifier-npme.json file) when running update-notifier
  2. Do not run update-notifier on prescribed installation (sudo npm i -g npme --unsafe) by using a hidden alias of the install command for the postinstall script
  3. Customize the update-notifier update message to say sudo npm i -g instead of npm i -g

The goal of item 2 is to check for updates when running the bin with valid non-autoinstall command (by decorating each command builder function) or with invalid/missing command (via custom yargs failure handler and strict mode).

I have tested this on an existing instance that already had npmo/npme installed, but I still need to test it against a fresh install instance. Will do that via next tag after merging this.

@nexdrew nexdrew changed the title Fix update notifier sudo Do not let update-notifier errors crash the bin Apr 27, 2016
@nexdrew
Copy link
Contributor Author

nexdrew commented Apr 28, 2016

Instead of silently ignoring the update-notifier crash, I opted to print a banner about the failure instead, with a note about how to fix it:

screen shot 2016-04-28 at 11 46 03 am

I'm not crazy about it, but I think keeping npme up-to-date is worth the annoyance.

@nexdrew nexdrew merged commit c28d4bb into master Apr 28, 2016
@nexdrew nexdrew deleted the fix-update-notifier-sudo branch April 28, 2016 15:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant