On expired `NEIL_GITHUB_TOKEN`, `neil dep upgrade` does nothing, then terminates with exit code 0
Observation
When I run neil dep upgrade either from the command line with an expired NEIL_GITHUB_TOKEN, Neil prints error messages, but returns with exit code 0.
$ neil-dev dep upgrade
{"message":"Bad credentials","documentation_url":"https://docs.github.com/rest","status":"401"}
{"message":"Bad credentials","documentation_url":"https://docs.github.com/rest","status":"401"}
$ echo $?
0
Expected behavior
I would expect neil dep upgrade to terminate with a nonzero exit code when nothing happened due to an expired Github token.
Perhaps also give a more helpful description than printing
{"message":"Bad credentials","documentation_url":"https://docs.github.com/rest","status":"401"}
twice, informing the user that their NEIL_GITHUB_TOKEN didn't appear to work.
Extra details
- This is only likely to hit Neil developers, users are not encouraged to setup their own github token for neil.
- This also appears to affect unit tests. I hit this first by writing a unit tests an not seeing anything update. (Details: https://github.com/babashka/neil/pull/236/files#diff-971ad24024ab537d1b6a3c2d0c37956be2ea8cdf52057b474f150b24d7c3b968R312-R326)
sounds good
Perhaps a better alternative could be to print a warning, then retry without authentication.
Preference, @borkdude?
sounds good as well!
Perhaps a better alternative could be to print a warning, then retry without authentication.
It appears this won't work.
From local testing, invalid Github token works from Github's API. The HTTP request only starts returning non-success HTTP responses after the free usage has been consumed. In other words, retrying won't help, we'll just trigger a different error; not helpful for the user.
I thought of something else that further complicates the issue. With the following deps.edn file, there are both Git deps and Clojars deps:
{:deps {babashka/pods {#_#_:local/root "/Users/borkdude/Dropbox/dev/clojure/babashka/babashka.pods"
:git/url "https://github.com/babashka/babashka.pods"
:git/sha "47e55fe5e728578ff4dbf7d2a2caf00efea87b1e"}
io.github.nextjournal/clerk {:mvn/version "0.10.560"}}
:aliases {}}
Current neil dep upgrade behavior is to "do the things we can": upgrade Clerk to latest, and leave babashka/pods alone because the HTTP request to Github failed.
I don't think changing this behavior to "Neil refuses to do anything because one request to Github failed" is helpful for the user.
Proposed new scope:
- Add an error message for invalid credentials
- Leave current exit code behavior as-is.