search

howdyai / botkit

Botkit is an open source developer tool for building chat bots, apps and custom integrations for major messaging platforms.
MIT License
11.38k stars 2.29k forks source link

Add Promise to botbuilder-adapter-webex both registerWebhookSubscription functions #2218

Open punithk opened 2 years ago

punithk commented 2 years ago

This is a fix for issue 2217.

The solution wraps the function with promise such that rejects are present and any thrown error can not be easily caught and handled by the user and should not affect existing bots.

Now developers can handle wrong access tokens this way without crashing during webhook subscription:

const adapter = new WebexAdapter({
     access_token: process.env.ACCESS_TOKEN, // access token from https://developer.webex.com
     public_address: process.env.PUBLIC_ADDRESS,  // public url of this app https://myapp.com/
     secret: process.env.SECRET // webhook validation secret - you can define this yourself
});

adapter.registerWebhookSubscription('/api/messages')
     .then()
     .catch(err => {    // <- this would fix the issue
            // Handle the error as per needs
     });

The changes should fix the issue and be backward compatible too.

ghost commented 2 years ago

CLA assistant check
All CLA requirements met.

Dayavats commented 2 years ago

Promisifying should resolve the issue of application breaking on wrong token case

Akanksha-270392 commented 2 years ago

Thanks for the suggestion. Looks like it will solve the issue.

VaniKaushik-2511 commented 2 years ago

The above suggestion is appreciated. It look like it will be the solution to the issue.

Maleeha456 commented 2 years ago

The solution mentioned above seems to resolve the issue.

Sayak-Bhattacharjee commented 2 years ago

It seems like this solution will solve the issue. Appreciated.

Dhananjay220398 commented 2 years ago

The solution mentioned above seems to resolve the issue.Thanks for the suggestion.

Adishjain58 commented 2 years ago

Thanks for the suggestion. Looks like it will solve the issue.

Akash2695 commented 2 years ago

Seems like this solution will resolve the issue, Thanks for the suggestion.

rashmi0403 commented 2 years ago

Thanks for the suggestion. Looks like it will solve the issue.

Sakshi21197 commented 2 years ago

I think @punithkrishnamurthy is doing right thing can we get this merged?

Deekshashandilya commented 2 years ago

facing similar issue while using this package. Please suggest some solution.