Skip to content
This repository has been archived by the owner on Jul 9, 2022. It is now read-only.

sendTypingIndicator: make callback optional #457

Merged
merged 1 commit into from
Apr 14, 2017
Merged

sendTypingIndicator: make callback optional #457

merged 1 commit into from
Apr 14, 2017

Conversation

ravkr
Copy link
Contributor

@ravkr ravkr commented Apr 6, 2017

I was getting this error:

Unhandled rejection TypeError: callback is not a function
    at /var/www/node/messenger/node_modules/facebook-chat-api/src/sendTypingIndicator.js:39:18
    at tryCatcher (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/promise.js:510:31)
    at Promise._settlePromiseAt (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/promise.js:584:18)
    at Promise._settlePromises (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/promise.js:700:14)
    at Async._drainQueue (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/async.js:123:16)
    at Async._drainQueues (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/async.js:133:10)
    at Immediate.Async.drainQueues [as _onImmediate] (/var/www/node/messenger/node_modules/facebook-chat-api/node_modules/bluebird/js/main/async.js:15:14)
    at processImmediate [as _immediateCallback] (timers.js:383:17)

when using:

end = api.sendTypingIndicator(event.threadID, function() {});
// run some background process
api.sendMessage(msg, event.threadID, end);

and it turned out that function end(cb) was called with cb being an object instead of a function.

@Schmavery
Copy link
Owner

Is this behaviour consistent with code you're seeing elsewhere in the api? I don't think I understand why we would want to allow people to pass in objects or other types that get silently swapped for an empty callback.
Do you think you could change this so that we explicitly fail when they pass something in that isn't a function? That seems to make more sense to me, if anything.

@Schmavery
Copy link
Owner

Ahh sorry, I reread the example and see what you were trying to do. Not sure what the right option is here. More of a design question at this point.

@ravkr
Copy link
Contributor Author

ravkr commented Apr 9, 2017

how about adding some warning when swapping for an empty callback?

@Schmavery
Copy link
Owner

Ok sure. I talked to @bsansouci and he thought this is a usecase worth supporting. Let's go with the warning rather than failing.

@ravkr ravkr force-pushed the master branch 2 times, most recently from ea8d6ff to adb280f Compare April 14, 2017 16:10
@ravkr ravkr changed the title sendTypingIndicator: make callback optional, fix end function sendTypingIndicator: make callback optional Apr 14, 2017
@ravkr
Copy link
Contributor Author

ravkr commented Apr 14, 2017

Is everything okey now?

Schmavery
Schmavery reviewed Apr 14, 2017
View reviewed changes
src/sendTypingIndicator.js Outdated
return function end(cb) {
makeTypingIndicator(false, threadID, cb || function() {});
if(!callback || (utils.getType(threadID) !== 'Function' && utils.getType(threadID) !== 'AsyncFunction')) {
cb = () => {};
Copy link
Owner

@Schmavery Schmavery Apr 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning here like above? Also I don't know why you're checking !callback and threadID instead of cb.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh god... I should get more sleep 😆

@ravkr
sendTypingIndicator: make callback optional
b32280d 
Added warning when there is a `callback` but it is not a function.
@Schmavery
Copy link
Owner

Thanks.

@Schmavery Schmavery merged commit 4ddc3fd into Schmavery:master Apr 14, 2017
how2945ard pushed a commit to how2945ard/facebook-chat-api that referenced this pull request May 30, 2017
@ravkr @Schmavery
sendTypingIndicator: make callback optional (Schmavery#457)
ac8de6c 
Added warning when there is a `callback` but it is not a function.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Schmavery Schmavery Schmavery left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ravkr @Schmavery