-
Notifications
You must be signed in to change notification settings - Fork 116
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
Lite-CI comments not posted #246
Comments
Sorry for the late response. Please let me know when you notice this again, and I'll do my best to dive into the bot logs quicker to see if there's any traces to that specific PR. The only thing I found now was this:
Although that looks related since it is trying to push a Jenkins status to GitHub, my first gut feeling leans towards https://github.com/nodejs/github-bot/issues/245 because that might make the bot try to find commits related to a PR that does not exist in the node repo. I'll ensure we don't get that error anymore, and see if we get more sensible errors logged in those scenarios, instead of crashing the process. |
While having a look at recent error logs, this fatal log stood out: ``` 02:48:11.721 FATAL bot: Unchaught exception, terminating bot process immediately TypeError: Cannot read property 'data' of undefined at githubClient.pullRequests.getCommits (/.../github-bot/lib/push-jenkins-update.js:92:29) at module.exports.sendError (/.../github-bot/node_modules/github/lib/index.js:874:7) at /.../github-bot/node_modules/github/lib/index.js:882:21 at callCallback (/.../github-bot/node_modules/github/lib/index.js:738:9) at IncomingMessage.<anonymous> (/.../github-bot/node_modules/github/lib/index.js:807:13) at IncomingMessage.emit (events.js:194:15) at endReadableNT (_stream_readable.js:1125:12) at process._tickCallback (internal/process/next_tick.js:63:19) ``` Since we never want these kinds of errors to happen since they end up killing the bot process, this fix is pushed to ensure we don't naively read the response from github.com until we know the request didn't fail for some reason. Refs #246
@phillipj should be easy to test, open a PR against node, trigger a lite ci. People might not notice failure, and if they do, might not know (or bother) to find this issue and comment. |
@phillipj I've just started https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/4000/ for nodejs/node#30007. |
Thanks! The only thing that got logged was related to the
Any easy way to know if anything has changed with the Lite-CI job lately, that could result in Jenkins not telling the github-bot about those jobs at all? |
While having a look at recent error logs, this fatal log stood out: ``` 02:48:11.721 FATAL bot: Unchaught exception, terminating bot process immediately TypeError: Cannot read property 'data' of undefined at githubClient.pullRequests.getCommits (/.../github-bot/lib/push-jenkins-update.js:92:29) at module.exports.sendError (/.../github-bot/node_modules/github/lib/index.js:874:7) at /.../github-bot/node_modules/github/lib/index.js:882:21 at callCallback (/.../github-bot/node_modules/github/lib/index.js:738:9) at IncomingMessage.<anonymous> (/.../github-bot/node_modules/github/lib/index.js:807:13) at IncomingMessage.emit (events.js:194:15) at endReadableNT (_stream_readable.js:1125:12) at process._tickCallback (internal/process/next_tick.js:63:19) ``` Since we never want these kinds of errors to happen since they end up killing the bot process, this fix is pushed to ensure we don't naively read the response from github.com until we know the request didn't fail for some reason. Refs #246
So, twice today when going to ci.nodejs.org, I've been bounced to a github page saying that jenkins has been doing excessive auth requests, and do I really want to allow it. I just push the yes button, but |
Yes, but it should be able to do 5000 requests/hr as described in API docs' rate limiting section which I naively assume should be more than enough. Currently no new related errors has been logged since the above. |
Refs: nodejs/node#29917 (comment)
on behalf of @Trott:
The text was updated successfully, but these errors were encountered: