Fallback argument missing: confusing error text

[saveman71]

Opening a new issue after reading slackapi/bolt-js#1249 which is closed.

@seratch, what was confusing on my end is that the error explicitly refers to a "`fallback` argument", so it leads to believe that the missing argument name is fallback, whereas it's actually text.

Maybe the warning message could be improved?

Suggestion:

The `text` argument is missing in the request payload for a chat.postMessage call - It's a best practice to always provide a `text` argument when posting a message which acts as a fallback. It is used in places where the content cannot be rendered such as: system push notifications, assistive technology such as screen readers, etc.

[filmaj]

Hey @saveman71,

Actually, the fallback parameter was a parameter for file attachments to a message. It is no longer applicable, though, as it is a legacy field. text is the appropriate fallback field to use.

The warning messages mentioned in this issue and in slackapi/bolt-js#1249 are not from bolt-js but came from Slack's responses to API requests. I just searched through our latest backend code and there are no more such messages returned, so I don't think that warning should be reported anymore. If you can reproduce it, can you please share code to be able to reproduce the warning?

[filmaj]

Hmm, as my colleague @seratch pointed out to me, some of these warnings come from the underlying @slack/web-api package: https://github.com/slackapi/node-slack-sdk/blob/%40slack/web-api%406.7.1/packages/web-api/src/WebClient.ts#L637-L664

Perhaps these warnings need a revision? I linked to the "legacy fields" documentation above - and since the fallback field is legacy, perhaps we should tweak this text to be more up to date with respect to the legacy fact? The text argument is the modern, correct way to provide a textual fallback to block messages as well as attachments. However, to @saveman71's point, that can be confusing based on the built warning from the node-slack-sdk.

I will move this issue over to the node-slack-sdk project as I think this can be an improvement that lands in that project (and bolt-js would get the benefit after updating its dependency).

[seratch]

@filmaj

Attachments are indeed marked as legacy in the documents but they are used a lot in the real world especially for having a color bar in a message. Thus, just suggesting the top-level text or removing the validation for fallback in attachments may not be the best way to revise the warning.

If we would like to improve the warning message to be clearer, the logic to check fallback existence can be:

• If the top-level text exists, we can skip warning for fallback in any of attachments
• If the top-level text does not exist and any of its attachments does not have fallback property, we still should warn it (we can say "You should have either top-level text or fallback in all the attachments" if we want to make it more accurate)
• No change to the use case where a message does not have attachments

Does this make sense to you? Also, if we do this, the same change should be done in other Python/Java SDKs.

[filmaj]

That makes a lot of sense! I think this is a great, small, but helpful improvement.