concierge / facebook

1 stars 1 forks source link

Error when someone is added or removed from group #2

Open p0358 opened 7 years ago

p0358 commented 7 years ago
error: "kpm_facebook" wywalił się. <unknown> jest teraz pokryty chorobą.
error: Error: TypeError: Cannot read property 'length' of undefined
    at /var/www/Concierge/modules/kpm_facebook/facebook.js:143:31
    at /var/www/Concierge/modules/kpm_facebook/node_modules/facebook-chat-api/src/listen.js:275:9
    at tryCatcher (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/promise.js:510:31)
    at Promise._settlePromiseAt (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/promise.js:584:18)
    at Promise._settlePromises (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/promise.js:700:14)
    at Async._drainQueue (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/async.js:123:16)
    at Async._drainQueues (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/async.js:133:10)
    at Immediate.Async.drainQueues (/var/www/Concierge/modules/kpm_facebook/node_modules/bluebird/js/main/async.js:15:14)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)
    at processImmediate [as _immediateCallback] (timers.js:617:5)

Not sure if this is really related to this module or upstream bug, perhaps facebook changed something again since there seem to be no updates on facebook-chat-api recently which could suddenly break this.

drkno commented 7 years ago

Thats an upstream bug, probably caused by a facebook change. I would suggest opening an issue there and ensuring that you not only have the lastest version of the api, but are using it too (easiest way is to remove node_modules/facebook-chat-api and modules/facebook/node_modules/facebook-chat-api, which will ensure the next run installs and uses the latest version). Old versions of this module just installed the api globally which can screw around with which version is required.

Schmavery commented 7 years ago

@mrkno as I mentioned on @p0358's issue on our repo, it seems like https://github.com/concierge/facebook/blob/master/facebook.js#L159 is accessing the old name for this value. Before now, we just served the event data directly from facebook, but now that they've changed the format, we process it on our end before sending it out so that it can stay more consistent. Sorry about the breaking change, we're trying to do a better job of managing them (it's hard when facebook changes under us) and recently introduced a Changelog document that we try to keep up to date so hopefully these issues will be easier to diagnose going forward.

drkno commented 7 years ago

@Schmavery no problem, as the latest gigantic PR on the main Concierge project demonstrates, I'm not unfamiliar with the reasoning behind breaking changes...

I've done a quick patch of this, @p0358 can you check if the problem still exists (at the very least to see if there is a bigger problem at play than an undefined variable). One day soon, I'll move this module to nice ES7 which could make readability of one of the oldest and most poorly written modules a bit nicer.

p0358 commented 7 years ago

Well, it doesn't crash the module anymore, but...

2017-07-12T19:54:02.707Z - error: Error while listening for facebook user events.
2017-07-12T19:54:02.717Z - debug: TypeError: usrs[i].split is not a function
    at _stopListeningMethod.api.listen.e (/var/www/Concierge/modules/facebook/facebook.js:171:63)
    at parsePackets (/var/www/Concierge/modules/facebook/node_modules/facebook-chat-api/src/listen.js:214:23)
    at Array.forEach (native)
    at /var/www/Concierge/modules/facebook/node_modules/facebook-chat-api/src/listen.js:109:12
    at tryCatcher (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/promise.js:510:31)
    at Promise._settlePromiseAt (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/promise.js:584:18)
    at Promise._settlePromises (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/promise.js:700:14)
    at Async._drainQueue (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/async.js:123:16)
    at Async._drainQueues (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/async.js:133:10)
    at Immediate.Async.drainQueues (/var/www/Concierge/modules/facebook/node_modules/bluebird/js/main/async.js:15:14)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)
    at processImmediate [as _immediateCallback] (timers.js:617:5)
drkno commented 7 years ago

That in itself isn't particularly surprising, I didn't have much time to test all of the changes I made. It should be relatively easy to fix if you want to have a go, and should only really require a one line change (or more if deleting the for loop, etc).

The intent of the breaking line is to extract all of the IDs from the event so we can add them to our list of known users.