Added API Key to Fire Function #89
Conversation
|
This pull request has been linked to Shortcut Story #50473: Decision SDK Fire Function Not Working. |
honeycomb-cheesecake
changed the title
src/client.ts
Outdated
| @@ -335,7 +341,7 @@ class PixelClient { | |||
| method: 'GET', | |||
| headers: { | |||
| 'X-Adzerk-Sdk-Version': this._versionString, | |||
| 'X-Kevel-ApiKey': additionalOpts?.apiKey, | |||
| 'X-Kevel-ApiKey': additionalOpts?.apiKey ?? this._options?.apiKey, | |||
There was a problem hiding this comment.
I'm not very familiar with the Decision API. The only thing I worry about here is that for the Decision client, we only include the header if it is present, but here, we are including the header in every request, with a null value if it is not present. I'm wondering if it might be safer to only include the header if the apikey is found?
There was a problem hiding this comment.
This was a comment from Ash to help the user experience. I'll roll it back to original functionality for now.
There was a problem hiding this comment.
Oh, I like the fallback from additionalOpts to this._options. I'm just afraid of sending X-Kevel-ApiKey: null rather than not sending the header at all if apikey is not set.
There was a problem hiding this comment.
It diverges against establshed behaviour, we could roll this, but we'd have to announce it.
There was a problem hiding this comment.
How about something like:
if (additionalOpts?.apiKey) {
opts.headers['X-Kevel-ApiKey'] = additionalOpts.apiKey;
}
There was a problem hiding this comment.
This wasn't actually the topic, but good suggestion as fetch actually still sends something even if set to undefined and null. Have updated.
There was a problem hiding this comment.
That was actually what I was trying to suggest, but Akira is a better communicator than me!
There was a problem hiding this comment.
Ahhhhhhh, I get it now. Sorry @vkurup that's clearly my misread.
There was a problem hiding this comment.
Heh, no worries! It's a 2 way street and both of ours have had heavy traffic on them recently
…pi key." This reverts commit 4858f8d.
Allows for authenticated requests.