Skip to content

Conversation

@goodhumored
Copy link

I just have basically changed all headers manually everywhere in code to match the rfc4229.

@goodhumored
Copy link
Author

But i broke most of the tests, need help

// now that all our headers are set, overwrite them if instructed.
for (var h in options.headers)
config.headers[h.toLowerCase()] = options.headers[h];
config.headers[h] = options.headers[h];
Copy link
Owner

Choose a reason for hiding this comment

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

This is a key part. We need to ensure that h is in PascalCase format, otherwise a user might pass a header in lowercase format which might not overwrite a previously set one.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, makes sense, i will add a formatting util function, probably take one from axios and add it here

But what do we do with tests? Why changing headers broke all the tests? I didnt dive much into this problems so i'm unsure

Copy link
Owner

@tomas tomas Aug 1, 2025

Choose a reason for hiding this comment

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

A quick look suggests that the changes broke a lookup either in the tests or in the code itself:

image

I'm happy to merge your PR but we need to make sure we don't break anything!

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.

2 participants