Bot failed to label PR #88

mscdex · 2016-10-21T13:14:57Z

phillipj · 2016-10-22T11:39:54Z

Logs says it the bot never got a pull_request.opened event from github for that PR, only pull_request.edited which is strange. Never seen this happen before, might be good to take notice when this occurs and maybe ask github support if we see more of this.

mscdex · 2016-10-28T05:33:49Z

Saw another instance just a bit ago: nodejs/node#9332

phillipj · 2016-10-28T09:17:09Z

#9332 seems to be related to the fact that we only fetch the first 100 labels of the node repository.

Pulled from the logs:

07:27:13.409 DEBUG bot: Filled existing repo labels cache: confirmed-bug,duplicate,help wanted,invalid,question,wontfix,in progress,tls,crypto,buffer,http,https,assert,util,cluster,child_process,dgram,c++,doc,debugger,dns,domain,events,fs,module,net,os,querystring,readline,repl,stream,timers,url,vm,zlib,windows,install,semver-minor,semver-major,discuss,build,V8,test,freebsd,path,meta,string_decoder,benchmark,openssl,npm,land-on-master,good first contribution,feature request,process,console,libuv,mentor-available,os x,arm,tools,land-on-v3.x,tty,intl,memory,land-on-v0.12,land-on-v0.10,land-on-v4.x,post-mortem,lts-landed-on-v4.x,lts-watch-v4.x,smartos,lts,ctc-agenda,ctc-meeting,v0.10,v0.12,v4.x,master,v5.x,lts-agenda,lib / src,lts-watch-v0.10,lts-watch-v0.12,land-on-v5.x,unconfirmed,stalled,mips,dont-land-on-v5.x,security,dont-land-on-v4.x,performance,error-handling,repro-exists,async_wrap,known issue test,promises,docs-requested,blocked,dont-land-on-v6.x,v6.x (req_id=d5d46425-aff7-411c-892b-bbacb61bec85, pr=nodejs/node/#9332, action=pull_request.opened)

cares is not part of the cached list of labels :/

mscdex · 2016-10-28T09:23:07Z

Ah ok

mscdex · 2016-11-02T20:38:03Z

nodejs/node#9422 was not automatically tagged as V8. The number of labels is at 100 exactly, so that shouldn't have been a problem?

phillipj · 2016-11-02T20:42:45Z

You're right, the V8 label existed in the labels cache at that moment, so that's not the issue for that PR. Related logs:

19:55:17.778 DEBUG bot: Filled existing repo labels cache: confirmed-bug,duplicate,help wanted,invalid,question,wontfix,in progress,tls,crypto,buffer,http,https,assert,util,cluster,child_process,dgram,c++,doc,debugger,dns,domain,events,fs,module,net,os,querystring,readline,repl,stream,timers,url,vm,zlib,windows,install,semver-minor,semver-major,discuss,build,V8,test,freebsd,path,meta,string_decoder,benchmark,openssl,npm,good first contribution,feature request,process,console,libuv,mentor-available,os x,arm,tools,tty,intl,memory,land-on-v4.x,post-mortem,lts-watch-v4.x,smartos,lts,ctc-agenda,ctc-meeting,v0.10,v0.12,v4.x,lib / src,lts-watch-v0.12,stalled,mips,security,dont-land-on-v4.x,performance,repro-exists,async_wrap,known issue test,promises,docs-requested,blocked,dont-land-on-v6.x,v6.x,cares,http_parser,V8_inspector,addons,punycode,v7.x,async_hooks,dont-land-on-v7.x,errors,ctc-review,lts-watch-v6.x,land-on-v6.x,constants (req_id=98d7aef2-e978-4a06-a74d-04416e91927c, pr=nodejs/node/#9422, action=pull_request.opened)
19:55:17.778 DEBUG bot: Resolved labels: v6.x (req_id=98d7aef2-e978-4a06-a74d-04416e91927c, pr=nodejs/node/#9422, action=pull_request.opened)
19:55:17.778 DEBUG bot: Trying to add labels: v6.x (req_id=98d7aef2-e978-4a06-a74d-04416e91927c, pr=nodejs/node/#9422, action=pull_request.opened)
19:55:17.947  INFO bot: Added labels: v6.x (req_id=98d7aef2-e978-4a06-a74d-04416e91927c, pr=nodejs/node/#9422, action=pull_request.opened)

mscdex · 2016-11-02T21:03:58Z

I think it may be because of the change in capitalization (to differentiate between node v8 and V8 the Google project)? The directory name is lowercase (deps/v8) but the label is V8.

phillipj · 2016-11-02T21:35:23Z

Yupp, agreed. My best guess is the node-repo.js#itemsInCommon(), made a simple script to verify it:

function itemsInCommon (arr1, arr2) {
  return arr1.filter((item) => arr2.indexOf(item) !== -1)
}

const existing = ['V8']
const resolved = ['v8']

console.log(itemsInCommon(existing, resolved))
// []

mscdex · 2016-11-02T22:37:07Z

Perhaps we could just lowercase everything when comparing?

mscdex · 2016-11-22T09:36:37Z

New case: nodejs/node#9737

phillipj · 2016-11-22T19:34:42Z

Yet another case of not getting the pull_request.opened webhook event from GitHub :/ This is what's in our logs related to that PR.

10:25:02.306  INFO bot: Emitting GitHub event (req_id=a5192783-813c-40f0-b969-8322e98822b1, pr=nodejs/node/#9737, action=pull_request.edited)
10:25:02.775  INFO bot: Emitting GitHub event (req_id=8dd41e3b-738e-452c-9e35-d6290bf8f717, pr=nodejs/node/#9737, action=pull_request.edited)
10:36:53.804  INFO bot: Emitting GitHub event (req_id=ac3724ad-4b3d-42ad-a1ee-874e85e429be, pr=nodejs/node/#9737, action=pull_request.labeled)
10:36:53.823  INFO bot: Emitting GitHub event (req_id=10749934-da31-4c00-8610-92e01c90f004, pr=nodejs/node/#9737, action=pull_request.labeled)

phillipj · 2016-11-23T21:00:19Z

Just realised we've got some deployment issues to resolve before blaming GitHub for not sending us all the events we should have gotten.

All current bot logs gets wiped whenever the bot is deployed
We've got some downtime when deploying -- its a matter of seconds, tho zero downtime deploys would ease my mind a little.

Both might be the reason we either don't see events logged, cause the logs are simply not complete or an even was pushed by GitHub while the bot was deploying.

mscdex · 2016-12-06T22:03:29Z

@phillipj Was there a deploy recently? nodejs/node#10156 is another example of no labels.

phillipj · 2016-12-07T20:43:23Z

Nope, no recent deploys. Looking at the systemd service supervising the bot, it might restarted two days ago. That should not have affected the PR 10156 tho since that's one day old.

No traces of PR 10156 in the logs either 😞

mscdex · 2016-12-07T20:52:19Z

Interesting... this time the repo's webhook logs show the "opened" notification was sent successfully (instead of failed in previous PRs) for that PR and it shows Express returned with 200 OK.

phillipj · 2016-12-07T21:14:13Z

Thanks for double checking that one, made me realise I did a mistake searching the logs previously. This got logged yesterday around the time it was opened and manually labeled:

21:06:03.030Z  INFO bot: Emitting GitHub event (req_id=e5f244f4-007c-4f86-ae3e-9938ba7bcb7e, pr=nodejs/node/#10156, action=pull_request.opened)
21:06:05.032Z DEBUG bot: Fetching PR files for labelling (req_id=e5f244f4-007c-4f86-ae3e-9938ba7bcb7e, pr=nodejs/node/#10156, action=pull_request.opened)
21:06:05.650Z DEBUG bot: Filled existing repo labels cache: addons,aix,arm,assert,async_hooks,async_wrap,benchmark,blocked,buffer,build,c++,cares,child_process,cluster,code-and-learn,confirmed-bug,console,crypto,ctc-agenda,ctc-review,debugger,dgram,discuss,dns,doc,domain,dont-land-on-v4.x,dont-land-on-v6.x,dont-land-on-v7.x,duplicate,errors,events,feature request,freebsd,fs,good first contribution,help wanted,http,https,http_parser,in progress,install,intl,invalid,land-on-v4.x,land-on-v6.x,lib / src,libuv,lts,lts-agenda,lts-watch-v0.12,lts-watch-v4.x,lts-watch-v6.x,macos,memory,mentor-available,meta,mips,module,net,notable-change,npm,openssl,os,path,performance,post-mortem,ppc,process,promises,punycode,querystring,question,readline,repl,repro-exists,s390,security,semver-major,semver-minor,smartos,stalled,stream,string_decoder,test,timers,tls,tools,tty,url,util,v0.12,v4.x,v6.x,v7.x,V8,V8_inspector,vm,windows,wontfix,zlib (req_id=e5f244f4-007c-4f86-ae3e-9938ba7bcb7e, pr=nodejs/node/#10156, action=pull_request.opened)
21:06:05.650Z DEBUG bot: Resolved labels:  (req_id=e5f244f4-007c-4f86-ae3e-9938ba7bcb7e, pr=nodejs/node/#10156, action=pull_request.opened)
22:02:48.045Z  INFO bot: Emitting GitHub event (req_id=8b1f34a4-6d01-4533-9106-837cbbcc4144, pr=nodejs/node/#10156, action=pull_request.labeled)
22:03:50.977Z  INFO bot: Emitting GitHub event (req_id=44658d79-4349-4853-b028-756835c8d84f, pr=nodejs/node/#10156, action=pull_request.labeled)

The most interesting is Resolved labels: logged from lib/node-repo.js#L35, meaning no labels was resolved from the filechanges and/or matched against existing repository labels..

I might have a look in a day or two, ideally reproducing it with a unit test in test/unit/node-labels.test.js. If someone wants to give it a shot in the meantime, please go ahead.

mscdex · 2016-12-07T22:43:50Z

I think I see the problem. PR incoming shortly...

mscdex · 2016-12-21T19:21:08Z

New one to look up: nodejs/node#10389

It was delivered successfully to the webhook with id:

X-GitHub-Delivery: 61767d80-c7a7-11e6-8500-f77cb4091d70

phillipj · 2016-12-26T22:43:32Z

@mscdex out of luck on this one, cause we're only keeping logs for 5 days. As of writing this, the oldest logs dates back to December 22th, meaning I was one day late for digging into the logs for PR 10389 -- sorry :/

mscdex · 2017-02-02T05:25:16Z

Another one: nodejs/node#11116

Successfully delivered with id: ea83e700-e905-11e6-9cb8-05aedd60f8fd

phillipj · 2017-02-02T07:23:48Z

Here's the logs for ^

Stripped the verbose attempt backport logs, except for one line about not landing cleanly on v7 as an FYI.

Shows the bot tried to add test. I assume that's not visible in the PR as it was labeled by the author immediately upon creation.

05:10:31.255Z  INFO bot: Emitting GitHub event (req_id=e562b508-c9cd-48da-a3a3-2a5510a3c659, pr=nodejs/node/#11116, action=pull_request.opened)
05:10:31.259Z  INFO bot: Emitting GitHub event (req_id=9fae2918-8f5e-48a6-973c-b7ce64244a4f, pr=nodejs/node/#11116, action=pull_request.labeled)
05:10:31.996Z DEBUG bot: Should have added (but temporary disabled): dont-land-on-v7.x (req_id=e562b508-c9cd-48da-a3a3-2a5510a3c659, pr=nodejs/node/#11116, action=pull_request.opened)
05:10:33.258Z DEBUG bot: Fetching PR files for labelling (req_id=e562b508-c9cd-48da-a3a3-2a5510a3c659, pr=nodejs/node/#11116, action=pull_request.opened)
05:10:33.458Z DEBUG bot: Resolved labels: test (req_id=e562b508-c9cd-48da-a3a3-2a5510a3c659, pr=nodejs/node/#11116, action=pull_request.opened)
05:10:33.637Z  INFO bot: Added labels: test (req_id=e562b508-c9cd-48da-a3a3-2a5510a3c659, pr=nodejs/node/#11116, action=pull_request.opened)

mscdex · 2017-02-02T08:00:22Z

@phillipj Right, but I've always seen it showing the bot adding them anyway. For example, just 5 hours prior to that another PR (nodejs/node#11113) was labelled in a similar way but it still shows the bot adding them. Interesting...

mscdex · 2017-02-07T00:04:55Z

Another: nodejs/node#11206
Successfully delivered with id: 9e47f680-ecb9-11e6-8685-ff8c5fd86487

EDIT: it actually looks like a labeling failure for some reason. I will look into that ...
EDIT 2: PR to fix this particular issue: #126

mscdex · 2017-03-21T18:23:11Z

Another PR: nodejs/node#11967
Successfully delivered with id: 7a4b0980-0e50-11e7-9861-c27e50e0c7bb

phillipj · 2017-03-22T07:26:37Z

Thanks! Yet another 404 from GitHub I'm afraid.

16:07:30.341Z DEBUG bot: Fetching PR files for labelling (req_id=bb46dba2-91b2-4712-b76b-5c11dbd30470, pr=nodejs/node/#11967, action=pull_request.opened)
16:07:30.523Z ERROR bot: Error retrieving files from GitHub (req_id=bb46dba2-91b2-4712-b76b-5c11dbd30470, pr=nodejs/node/#11967, action=pull_request.opened)
    err: {
      "code": 404,
      "status": "Not Found",
      "message": "{\"message\":\"Not Found\",\"documentation_url\":\"https://developer.github.com/v3\"}"
    }

mscdex · 2017-03-22T07:30:30Z

@phillipj Could we add some kind of retry mechanism in case of 404? Like x retries with y delay between each retry? I'm guessing it's just a race condition anyway ...

phillipj · 2017-03-22T07:32:39Z

Yeah, sounds like a good idea 👍

phillipj · 2017-05-06T17:21:42Z

Closing this somewhat general "failed to label" issue for now, better off with more focused issues per failed scenario.

phillipj mentioned this issue Nov 13, 2016

V8 changes not labeled correctly #92

Closed

phillipj mentioned this issue Nov 23, 2016

github-bot: redirect stdout to file nodejs/build#553

Closed

mscdex mentioned this issue Dec 7, 2016

labels: fix build label #99

Merged

phillipj mentioned this issue Mar 22, 2017

labels: retry request for PR files if GitHub request fails #130

Merged

phillipj closed this as completed May 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bot failed to label PR #88

Bot failed to label PR #88

mscdex commented Oct 21, 2016

phillipj commented Oct 22, 2016

mscdex commented Oct 28, 2016

phillipj commented Oct 28, 2016

mscdex commented Oct 28, 2016

mscdex commented Nov 2, 2016

phillipj commented Nov 2, 2016

mscdex commented Nov 2, 2016 •

edited

Loading

phillipj commented Nov 2, 2016

mscdex commented Nov 2, 2016

mscdex commented Nov 22, 2016

phillipj commented Nov 22, 2016

phillipj commented Nov 23, 2016

mscdex commented Dec 6, 2016

phillipj commented Dec 7, 2016

mscdex commented Dec 7, 2016 •

edited

Loading

phillipj commented Dec 7, 2016

mscdex commented Dec 7, 2016

mscdex commented Dec 21, 2016 •

edited

Loading

phillipj commented Dec 26, 2016

mscdex commented Feb 2, 2017

phillipj commented Feb 2, 2017

mscdex commented Feb 2, 2017

mscdex commented Feb 7, 2017 •

edited

Loading

mscdex commented Mar 21, 2017

phillipj commented Mar 22, 2017

mscdex commented Mar 22, 2017

phillipj commented Mar 22, 2017

phillipj commented May 6, 2017

Bot failed to label PR #88

Bot failed to label PR #88

Comments

mscdex commented Oct 21, 2016

phillipj commented Oct 22, 2016

mscdex commented Oct 28, 2016

phillipj commented Oct 28, 2016

mscdex commented Oct 28, 2016

mscdex commented Nov 2, 2016

phillipj commented Nov 2, 2016

mscdex commented Nov 2, 2016 • edited Loading

phillipj commented Nov 2, 2016

mscdex commented Nov 2, 2016

mscdex commented Nov 22, 2016

phillipj commented Nov 22, 2016

phillipj commented Nov 23, 2016

mscdex commented Dec 6, 2016

phillipj commented Dec 7, 2016

mscdex commented Dec 7, 2016 • edited Loading

phillipj commented Dec 7, 2016

mscdex commented Dec 7, 2016

mscdex commented Dec 21, 2016 • edited Loading

phillipj commented Dec 26, 2016

mscdex commented Feb 2, 2017

phillipj commented Feb 2, 2017

mscdex commented Feb 2, 2017

mscdex commented Feb 7, 2017 • edited Loading

mscdex commented Mar 21, 2017

phillipj commented Mar 22, 2017

mscdex commented Mar 22, 2017

phillipj commented Mar 22, 2017

phillipj commented May 6, 2017

mscdex commented Nov 2, 2016 •

edited

Loading

mscdex commented Dec 7, 2016 •

edited

Loading

mscdex commented Dec 21, 2016 •

edited

Loading

mscdex commented Feb 7, 2017 •

edited

Loading