notify: update notify/wechat

[binacs]

Accoding to WeChat Work API, we will get response such as:

{
   "errcode": 0,
   "errmsg": "ok",
   "access_token": "accesstoken000001",
   "expires_in": 7200
}

instead of:

{
   "code": 0,
   "error": "ok",
   "access_token": "accesstoken000001",
}

And sometimes the request fails because it uses an expired token

So I did these things to update the code:

1. Update weChatResponse and tokenResponse;
2. Use expireIn instead of 2 * time.Hour;
3. Record the time before we send the request, not after we receive the response;

For test, I wrote the following code:

func TestNotify(t *testing.T) {
        // Use `*` to protect information
	secret := "d2mS-c3*********************************dwKLMbDI"
	u, err := url.Parse("https://qyapi.weixin.qq.com/cgi-bin/")
	require.NoError(t, err)
	n, err := New(
		&config.WechatConfig{
			APIURL:      &config.URL{URL: u},
			HTTPConfig:  &commoncfg.HTTPClientConfig{},
			ToUser:      "binacs",
			AgentID:     "*********",
			CorpID:      "******************",
			MessageType: "text",
			Message:     "binacs: send from prometheus/alertmanager unit test",
			APISecret:   config.Secret(secret),
		},
		test.CreateTmpl(t),
		log.NewJSONLogger(os.Stdout),
	)
	require.NoError(t, err)
	ok, err := n.Notify(context.Background(), []*types.Alert{
		&types.Alert{
			Alert: model.Alert{
				Labels: model.LabelSet{
					"lbl1": "val1",
				},
				StartsAt: time.Now(),
				EndsAt:   time.Now().Add(time.Hour),
			},
		},
	}...)
	t.Log(ok, err)
}

And it works:

Thanks for your review.

[simonpasquier]

Thanks, just a few remarks! Could you add tests too please?

[simonpasquier]

It may be safer to renew the token before it expires (for instance when it reached 90% of its validity).

[simonpasquier]

instead of storing the creation time of the token and its period, you could record its expiration time (minus a safety interval as described above).

[binacs]

Thank you very much for your advice!

I have submitted the latest code. Could you please review it again? @simonpasquier

And the unit test works like this:

[simonpasquier]

Suggested change

	expireAt time.Time
	tokenRenewAt time.Time

[simonpasquier]

The unit test can't access the real WeChat endpoint. We need to mock the WeChat API to simulate the various responses. Since it isn't trivial, I can have a look at it. Also this code doesn't test anything.

[SoloJacobs]

Hi @binacs ,

it appears to me that we have not heard back from you after the latest review. In the meantime, a fix has been submitted in #3330 .

Hopefully, this addresses the issue. If not, I'm happy to review a PR, which is based of the state of main. Thank you for your contribution.

Kind regards