superfaceai / one-sdk-js

1️⃣ One Node.js SDK for all the APIs you want to integrate with
https://superface.ai
MIT License
47 stars 3 forks source link

Fix for resolving accept header #290

Closed freaz closed 2 years ago

freaz commented 2 years ago

Description

Do not set accept header when defined in HTTP request headers. I wanted to normalize headers and work with them, but that could lead to unexpected behavior and better will be to allow set as many headers and in any case. It will delegate need to normalize to client and server.

One thing I am not happy about, is how the logic is split in map-interpreter and filters.

Motivation and Context

Fixes #264

Types of changes

Checklist:

freaz commented 2 years ago

I don't think it is entirely wrong, but I would just like to see headers resolution in map-interpreter instead of filters.

But more importantly, when I looked at PR without focus on Accept header, I realized we work with case-sensitive keys for header names, and use of content-type and Content-Type can lead to interesting bugs.

freaz commented 2 years ago

I would merge this one, and apply similar approach for content-type and user-agent in headersFilter.