Return response metadata with chat errors

[StupidScience]

Description

Currently with methods like SendMessageContext there is no way to get info about error like it is possible for OpenViewContext as example. So it leads that you just get errors like invalid_blocks without any additional information while in real response can be something like

{
    "ok": false,
    "error": "invalid_blocks",
    "errors": [
        "failed to match all allowed schemas [json-pointer:/blocks/1/elements/0/text]",
        "invalid additional property: verbatim [json-pointer:/blocks/1/elements/0/text]"
    ],
    "response_metadata": {
        "messages": [
            "[ERROR] failed to match all allowed schemas [json-pointer:/blocks/1/elements/0/text]",
            "[ERROR] invalid additional property: verbatim [json-pointer:/blocks/1/elements/0/text]"
        ]
    }
}

I've tested it with invocation like this:

s.SendMessageContext(
    context.TODO(),
    channelID,
    slack.MsgOptionText("", false),
    slack.MsgOptionBlocks(
	  slack.NewSectionBlock(
		  slack.NewTextBlockObject(slack.MarkdownType, "test", true, true),
		  nil,
		  nil,
	  ),
  ),
)

So it is really annoying to debug this issues in semi-manual way to understand what is going on in real.

May be there is some option to create some kind of error wrapper type like the following to avoid breaking the compatibility:

type Error struct {
	Err             string
	Messages []string
}

Or return response from specified function so it would be possible to get this information?

(Optional) Slack's documentation

https://api.slack.com/changelog/2016-09-28-response-metadata-is-on-the-way

[StupidScience]

@kanata2 sorry for bothering. what do you think about this?

[kanata2]

Sorry for my slow response 😩
I'll confirm later.

[abustany]

one could imagine a SlackErrorResponse, which would basically be a SlackResponse with Ok == false. That struct would implement the error interface returning its Error field as the current code does, but could also be casted using errors.As to get access to ResponseMetadata. Something like

// SlackErrorResponse is a SlackResponse with an error set
type SlackErrorResponse struct {
  Error string
  ResponseMetadata ResponseMetadata
}

func (r SlackErrorResponse) Error() string { return r.Error } // keeps the same behavior as before

// we'd change this method
func (t SlackResponse) Err() {
  if t.Ok { return nil } // unchanged
  if strings.TrimSpace(t.Error) == "" { return nil } // unchanged
  return SlackErrorResponse{Error: t.Error, ResponseMetadata: t.ResponseMetadata}
}

I'm happy to work on implementing this if you think it makes sense.

[abourget]

yes please please, same issue here!

[abourget]

Another option:

type SlackErrorResponse struct {
  Response SlackResponse
}
func (e SlackErrorResponse) Error() string {
  return e.Response.Error
}

and we could even add a few newer fields to SlackResponse:

type SlackResponse struct {
	Ok               bool             `json:"ok"`
	Error            string           `json:"error"`
	Errors []string `json:"errors"`
        Warnings []string `json:"warnings"`
	ResponseMetadata ResponseMetadata `json:"response_metadata"`
}

That would future-proof the returned Error object.

[abourget]

Yet another option: append the ResponseMetadata.Messages array, joined with a ", " to the currently returned errors.New() here:

func (t SlackResponse) Err() error {
	if t.Ok {
		return nil
	}
...
	return errors.New(t.Error)
}

This way, everyone benefits from clearer messages.

[abourget]

Third option, a mix of both two previous options: return a clearer error message on the SlackErrorResponse's proposed Error() string function. Get the best of all the worlds.

[kanata2]

Sorry for delay.
I generally agree with @abustany's original suggestion.
I would like to prioritize the review of other pull requests,
so if possible, could you please submit a pull request the proposal here?

@abourget @abustany
Thank you for your great suggestions.