Generalize User-Agent header in receiver requests #2730
Conversation
Signed-off-by: István Gazsi <[email protected]>
We do changelogs at release time. |
| return true, err | ||
| } | ||
|
|
||
| req.Header.Set("Content-Type", "application/json") |
There was a problem hiding this comment.
As I understand for GET requests there is no need for a Content-Type header. I've also tried to check other libraries targeting the WeChat API and the API documentation, and I wasn't able to find any mention of this. Unfortunately I cannot test this in practice myself.
But I'd gladly restore the old approach in this case if you think it's too risky to remove.
There was a problem hiding this comment.
You are probably right, but we should confirm this by an manual test against wechat.
There was a problem hiding this comment.
I think @theag3nt is right, the Content-Type header does not apply to GET requests. Its a bit convoluted but according to [1] the header applies to a requests body, which GET doesn't have. I'd argue setting the header before was probably a bug, the header is probably ignored.
Furthermore, this request is only for refreshing the access token. The actual alert requests happens further down, is a POST requests and sets the Content-Type.
So I'd be 👍 to move forward.
There was a problem hiding this comment.
Also https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type is a bit more explicit/
|
Thanks! |
…rometheus#2730) Signed-off-by: István Gazsi <[email protected]>
…rometheus#2730) Signed-off-by: István Gazsi <[email protected]> Signed-off-by: Marko Posavec <[email protected]>
The request helper functions in the
notifypackage now set the user agent for all requests. All applicable receivers were transitioned to these helpers, except opsgenie. In the case of opsgenie where requests are pre-generated the user agent is set before sending each request.Resolves #2722