Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Update channelServiceRoutes to add next() parameter to support restify 10.0.0+ #4429

Merged
merged 4 commits into from
Feb 16, 2023

Conversation

anishprasad01
Copy link
Contributor

@anishprasad01 anishprasad01 commented Feb 9, 2023

#minor
Fixes Internal: https://portal.microsofticm.com/imp/v3/incidents/details/365652257/home

Description

restify 10.0.0 and greater enforces the need for a next() callback that passes execution to the next function in the handler chain. channelServiceRoutes.ts does not currently address this requirement. Since this class is required to interact with skills, this means that root bots that import this class will fail to start.

Specific Changes

  • Add the next: Function parameter to the handlers in channelServiceRoutes.ts
  • Add a return next() statement to the aforementioned handlers
  • Update the tests in channelServiceRoutes.test.js to pass in an empty Function as next in each test

Testing

Ran the updated unit tests successfully.

@anishprasad01 anishprasad01 added customer-reported Issue is created by anyone that is not a collaborator in the repository. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. Area: Skills The issue is related to skills Automation: No parity PR does not need to be applied to other languages. labels Feb 9, 2023
@anishprasad01 anishprasad01 self-assigned this Feb 9, 2023
@anishprasad01 anishprasad01 changed the title Update channelServiceRoutes to add next() parameter to support restify 10.0.0+ fix: Update channelServiceRoutes to add next() parameter to support restify 10.0.0+ Feb 9, 2023
@coveralls
Copy link

coveralls commented Feb 9, 2023

Pull Request Test Coverage Report for Build 4168147444

  • 12 of 13 (92.31%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.002%) to 84.653%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botbuilder/src/channelServiceRoutes.ts 12 13 92.31%
Files with Coverage Reduction New Missed Lines %
libraries/botbuilder-lg/src/templateExtensions.ts 1 84.48%
Totals Coverage Status
Change from base Build 4128392066: -0.002%
Covered Lines: 20013
Relevant Lines: 22404

💛 - Coveralls

@johnataylor johnataylor self-requested a review February 10, 2023 19:41
@johnataylor
Copy link
Member

we need to first verify that this change doesn't break existing Node and Restify dependencies

@anishprasad01
Copy link
Contributor Author

we need to first verify that this change doesn't break existing Node and Restify dependencies

Tested the code as of most recent commit in Sample 81 with Node 16.19.0 + restify 8.6.1 and Node 18.14.0 + restify 10.0.0. Both ran successfully. Unit tests also passing.

@anishprasad01 anishprasad01 marked this pull request as ready for review February 13, 2023 22:34
@anishprasad01 anishprasad01 requested a review from a team as a code owner February 13, 2023 22:34
@anishprasad01 anishprasad01 merged commit f9cf5fd into main Feb 16, 2023
@anishprasad01 anishprasad01 deleted the anishprasad01/restify-10-compatibility-update branch February 16, 2023 22:36
tracyboehrer pushed a commit that referenced this pull request Mar 21, 2023
…estify 10.0.0+ (#4429)

* Add next callback to support restify chain handler

* Update next type and channelServiceRoutes tests

* Remove return next() to fix Headers Sent Error

* Add return next() in the handler function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Skills The issue is related to skills Automation: No parity PR does not need to be applied to other languages. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-reported Issue is created by anyone that is not a collaborator in the repository.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants