Strider-CD / strider-github

Github provider for strider
25 stars 34 forks source link

Problems in GitHub communication #17

Closed oliversalzburg closed 9 years ago

oliversalzburg commented 10 years ago

Today I noticed that our strider is no longer processing commits from our projects.

When looking into the issue, I noticed some problems which were apparently fixed by https://github.com/Strider-CD/strider-github/pull/16 (please but another release to NPM btw). Even after I applied those changes to our install, I still see the following response on GitHub:

TypeError: Cannot read property 'author' of undefined
    at pushJob (/home/deploy/strider/node_modules/strider-github/lib/webhooks.js:140:19)
    at startFromCommit (/home/deploy/strider/node_modules/strider-github/lib/webhooks.js:42:16)
    at receiveWebhook (/home/deploy/strider/node_modules/strider-github/lib/webhooks.js:222:3)
    at callbacks (/home/deploy/strider/node_modules/express/lib/router/index.js:164:37)
    at projectProvider (/home/deploy/strider/lib/middleware.js:107:3)
    at callbacks (/home/deploy/strider/node_modules/express/lib/router/index.js:164:37)
    at Promise.<anonymous> (/home/deploy/strider/lib/middleware.js:201:5)
    at Promise.<anonymous> (/home/deploy/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)
    at Promise.EventEmitter.emit (events.js:95:17)
    at Promise.emit (/home/deploy/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:79:38)

I see that error for the test request that GitHub sends when you add a new hook.

laurentj commented 10 years ago

I also have this kind of issue after a push on github:

TypeError: Cannot call method 'indexOf' of undefined
    at pushJob (/home/strider/strider/node_modules/strider-github/lib/webhooks.js:126:19)
    at startFromCommit (/home/strider/strider/node_modules/strider-github/lib/webhooks.js:42:16)
    at receiveWebhook (/home/strider/strider/node_modules/strider-github/lib/webhooks.js:222:3)
    at callbacks (/home/strider/strider/node_modules/express/lib/router/index.js:164:37)
    at projectProvider (/home/strider/strider/lib/middleware.js:107:3)
    at callbacks (/home/strider/strider/node_modules/express/lib/router/index.js:164:37)
    at Promise.<anonymous> (/home/strider/strider/lib/middleware.js:201:5)
    at Promise.<anonymous> (/home/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)
    at Promise.EventEmitter.emit (events.js:95:17)
    at Promise.emit (/home/strider/strider/node_modules/mongoose/node_modules/mpromise/lib/promise.js:79:38)

So it is caused by this line:

if (payload.ref.indexOf('refs/heads/') === 0) {
    branchname = payload.ref.substring('refs/heads/'.length)
    ref = {
      branch: branchname,
      id: payload.after
    }
  } else {

although, in the payload in the request, I have the ref property

  "ref": "refs/heads/jelix-1.6.x",
oliversalzburg commented 10 years ago

@laurentj That was fixed by https://github.com/Strider-CD/strider-github/pull/16 Unless I'm mistaken :)

laurentj commented 10 years ago

@oliversalzburg yes, you're right. I didn't saw the fix about JSON.parse...

niallo commented 10 years ago

Latest HEAD of strider-github is now on npm as v1.2.2. But you can confirm that there is still an issue with Github web hooks?

oliversalzburg commented 10 years ago

@niallo Our setup seems to work fine now. I think the problem I was seeing is only related to that initial ping sent by GitHub when the webhook is set up.

cusspvz commented 10 years ago

Same error this side, maybe npm version is outdated?

knownasilya commented 9 years ago

@cusspvz did you solve the issue?

cusspvz commented 9 years ago

Yes, I don't quite remember this but things are working flawless at the time.

knownasilya commented 9 years ago

Thanks @cusspvz