Skip to content
This repository was archived by the owner on Sep 20, 2024. It is now read-only.

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

Open
punithk wants to merge 1 commit into
howdyai:mainfrom
punithk:webex_adapter_uncatchable_throws
Open

Add Promise to botbuilder-adapter-webex both registerWebhookSubscription functions#2218
punithk wants to merge 1 commit into
howdyai:mainfrom
punithk:webex_adapter_uncatchable_throws

Conversation

@punithk

@punithk punithk commented May 20, 2022

Copy link
Copy Markdown

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

ghost commented May 20, 2022

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@Dayavats

Copy link
Copy Markdown

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

@Akanksha-270392

Akanksha-270392 commented May 24, 2022

Copy link
Copy Markdown

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

@VaniKaushik-2511

VaniKaushik-2511 commented May 25, 2022

Copy link
Copy Markdown

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

@Maleeha456

Copy link
Copy Markdown

The solution mentioned above seems to resolve the issue.

@Sayak-Bhattacharjee

Copy link
Copy Markdown

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

@Dhananjay220398

Copy link
Copy Markdown

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

@Adishjain58

Copy link
Copy Markdown

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

@Akash2695

Copy link
Copy Markdown

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

@rashmi0403

Copy link
Copy Markdown

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

@Sakshi21197

Copy link
Copy Markdown

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

@Deekshashandilya

Copy link
Copy Markdown

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.