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(node-http-handler): fix invalid usage of parsed url #1408

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

Defman
Copy link
Contributor

@Defman Defman commented Sep 19, 2024

issue

Description of changes:

Nodejs URL is not fully compatible with http.RequestOptions see nodejs/node#39738


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Defman Defman requested review from a team as code owners September 19, 2024 18:10
@Defman Defman force-pushed the urlToHttpOptions branch 2 times, most recently from 5810491 to 4ae8460 Compare September 20, 2024 07:40
@Defman
Copy link
Contributor Author

Defman commented Sep 20, 2024

@kuhe is this something you could have a look at?

@kuhe kuhe merged commit 08fbedf into smithy-lang:main Sep 20, 2024
11 checks passed
@kuhe
Copy link
Contributor

kuhe commented Sep 20, 2024

@Defman
Copy link
Contributor Author

Defman commented Sep 20, 2024

Any chance of getting it bumped in https://github.com/aws/aws-sdk-js-v3 ?

@kuhe
Copy link
Contributor

kuhe commented Sep 20, 2024

yes, but it will happen at a later date. A lockfile update or other type of fresh install would bring in the fix.

@Defman
Copy link
Contributor Author

Defman commented Sep 21, 2024

Yea, I feel a bit stupid right now... had been trying to get my fork to work with npm overrides all day... however after it got merged I should of course just run npm upgrade @smithy/node-http-handler to update the lock file...

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.

3 participants