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: botFrameworkClientFetchImpl function missing headers from Zod validation #3959

Merged
merged 4 commits into from
Oct 26, 2021

Conversation

sw-joelmut
Copy link
Collaborator

Fixes #minor

Description

This PR fixes an issue when the Host wants to send an Activity to the Skill using the internal botFrameworkClientFetchImpl function to make the POST request.
This validation error was found as part of the issue BotBuilder-Samples-#3526 to add support for MSI to the JavaScript samples, that currently is under development.

Specific Changes

  • When the Host tries to call the Skill using the internal botFrameworkClientFetchImpl function, the zod validation fails to recognize the keys from the headers object.

Testing

The following images show the error and the bots working after the fix.
image
image

@sw-joelmut sw-joelmut requested a review from a team as a code owner October 20, 2021 17:19
@coveralls
Copy link

coveralls commented Oct 20, 2021

Pull Request Test Coverage Report for Build 1381148946

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 84.586%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botframework-connector/src/auth/botFrameworkClientImpl.ts 0 1 0.0%
Totals Coverage Status
Change from base Build 1347405782: 0.007%
Covered Lines: 19707
Relevant Lines: 22074

💛 - Coveralls


const response = await axios.post(url, JSON.parse(body), {
headers: z.record(z.string()).parse(init.headers ?? {}),
headers: z.record(z.string()).parse(headers ?? {}),
Copy link
Contributor

Choose a reason for hiding this comment

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

headers should already be typed properly from the changes on line 16, so you should just be able to do { headers }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, @joshgummersall, feedback applied!

@joshgummersall joshgummersall merged commit 3d0e50d into main Oct 26, 2021
@joshgummersall joshgummersall deleted the southworks/fix/zod-headers-validation branch October 26, 2021 15:59
joshgummersall pushed a commit that referenced this pull request Oct 26, 2021
…lidation (#3959)

* Fix Zod missing headers validation error

* Remove nullish coalescing operator for headers

* Clean up botFrameworkClientFetchImpl headers validation

* Update botFrameworkClientImpl.ts

Co-authored-by: MartLuc <[email protected]>
joshgummersall pushed a commit that referenced this pull request Oct 28, 2021
#3963)

* fix: botFrameworkClientFetchImpl function missing headers from Zod validation (#3959)

* Fix Zod missing headers validation error

* Remove nullish coalescing operator for headers

* Clean up botFrameworkClientFetchImpl headers validation

* Update botFrameworkClientImpl.ts

Co-authored-by: MartLuc <[email protected]>

* fix: add more release branches

Co-authored-by: Joel Mut <[email protected]>
Co-authored-by: MartLuc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants