Fail on errors returned from the Slack API when using API Token

[liamnichols]

Checklist

• I've read and followed the Contribution Guidelines
• step.yml and README.md is updated with the changes (if needed)

Version

Requires a PATCH version update

Context

In #44 the Step was updated to support both legacy (fire and forget) webhooks as well as the chat.postMessage API method that is part of the Slack API (via the API Token). With this, there is a bit of a change of how errors are reported.

The legacy webhook would just fail if it wasn't valid and generally a HTTP 200 response indicated that it sent the message, which lines up with the logic that currently exists:

steps-slack-message/main.go

Lines 154 to 160 in b38da23

	if resp.StatusCode != http.StatusOK {
	body, err := ioutil.ReadAll(resp.Body)
	if err != nil {
	return fmt.Errorf("server error: %s, failed to read response: %s", resp.Status, err)
	}
	return fmt.Errorf("server error: %s, response: %s", resp.Status, body)
	}

However the Slack API does not work in the same way. Failures to send messages still result in a HTTP 200 response and instead the status is described as part JSON response. For example, if we make a request with an invalid token:

$ curl -X POST \ 
  -i \
  -H 'Authorization: Bearer BAD_AUTH' \
  -H 'Content-type: application/json; charset=utf-8' \
  --data '{"channel":"foo","text": "bar"}' \
  https://slack.com/api/chat.postMessage

HTTP/2 200 
date: Wed, 22 Feb 2023 21:17:57 GMT
server: Apache
x-powered-by: HHVM/4.153.1
access-control-allow-origin: *
referrer-policy: no-referrer
x-slack-backend: r
x-slack-unique-id: Y_aGhYqByRwZTLeYlWkr_QAAAB0
strict-transport-security: max-age=31536000; includeSubDomains; preload
access-control-allow-headers: slack-route, x-slack-version-ts, x-b3-traceid, x-b3-spanid, x-b3-parentspanid, x-b3-sampled, x-b3-flags
access-control-expose-headers: x-slack-req-id, retry-after
expires: Mon, 26 Jul 1997 05:00:00 GMT
cache-control: private, no-cache, no-store, must-revalidate
pragma: no-cache
x-robots-tag: noindex,nofollow
x-xss-protection: 0
x-content-type-options: nosniff
x-slack-req-id: c0625637e45fd91493a372dd0035be76
vary: Accept-Encoding
content-type: application/json; charset=utf-8
x-envoy-upstream-service-time: 101
x-backend: main_normal main_canary_with_overflow main_control_with_overflow
x-server: slack-www-hhvm-main-iad-czbn
x-slack-shared-secret-outcome: no-match
via: envoy-www-iad-cakx, envoy-edge-fra-jcnw
x-edge-backend: envoy-www
x-slack-edge-shared-secret-outcome: no-match

{"ok":false,"error":"invalid_auth"}%    

With the current implementation this means that Bitrise will continue as if the Slack message had sent successfully even if it didn't. This coupled with Slack apps being easy to misconfigure means that we can often end up spending a lot of extra time trying to work out what went wrong when it would be really helpful to have that error printed to us.

Some other use cases that I ran into during my time configuring things:

1. My api_token was invalid (invalid_auth)
2. Bot wasn't invited to the channel (channel_not_found)
3. Bot didn't have the right permission to send a message (invalid_scopes)

Changes

To improve this, my aim is to always attempt to parse the JSON response when using api_token. If ok is false, we fail and include the error in the message.

While #82 updated the step to start parsing the response, it would only do it conditionally if the output_thread_ts was set so I have also made additional changes to account for this.

As part of this change, instead of erroring if you incorrectly use output_thread_ts and webhook_url after* the request is sent, I moved the check into the existing validate method that runs after loading the Config.

Investigation details

Decisions

[ohlulu]

Is there any progress on this PR?
I wasted several hours because I forgot to add the bot to the channel.
@liamnichols Although the PR wasn't merged, thank you very much for your contribution.