node-slack-sdk icon indicating copy to clipboard operation
node-slack-sdk copied to clipboard

fallback always renders even if attachments are included

Open edwmurph opened this issue 3 years ago • 4 comments

Packages:

Select all that apply:

  • [x] @slack/web-api

Reproducible in:

The Slack SDK version

6.7.2

Node.js runtime version

16.16.0

OS info

ProductName:	macOS
ProductVersion:	12.5.1
BuildVersion:	21G83
Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:35 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T8101

Steps to reproduce:

run this snippet to repro:
const { WebClient }               = require('@slack/web-api');
const { decrypt }                 = require('enigma');
const { channel, botTokenCipher } = require('amp/lib/config').slack.dev;

const blocks = [
  {
    'type': 'header',
    'text': {
      'type': 'plain_text',
      'text': 'header 1'
    }
  },
  {
    'type': 'context',
    'elements': [
      {
        'type': 'mrkdwn',
        'text': '*ctx1:* val1'
      },
      {
        'type': 'mrkdwn',
        'text': '*ctx2:* val2'
      }
    ]
  }
];

const main = async() => {
  const token = await decrypt( botTokenCipher );
  const slack = new WebClient( token );

  await slack.chat.postMessage({
    channel,
    text: 'fallback1',
    attachments: [
      {
        fallback: 'fallback2',
        color: '#ff0000',
        blocks
      }
    ]
  });
};

main()
.then( console.log, console.log )
.finally( () => process.exit( 0 ) );

it renders the fallback even though the attachments are included

image
and then when i remove the top level `text` opt
  await slack.chat.postMessage({
    channel,
    attachments: [
      {
        fallback: 'fallback2',
        color: '#ff0000',
        blocks
      }
    ]
  });
it no longer renders the fallback message, but now i get a warning in my console:
[WARN]  web-api:WebClient:0 The top-level `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. The `text` is used in places where the content cannot be rendered such as: system push notifications, assistive technology such as screen readers, etc.

Expected result:

i need some way to post a slack message with attachments and with a fallback without the fallback rendering and without a console warning

Actual result:

either i get a warning in the console about not providing a fallback or the fallback is incorrectly posted with the message, despite including an attachment

see previous section for more context

Notes

related https://github.com/slackapi/node-slack-sdk/issues/1476

edwmurph avatar Sep 01 '22 21:09 edwmurph

Hi @edwmurph, thanks for sharing the detailed report on the issue. In your use case, I agree that text can be skipped as the attachment has the fallback property. At #1476 discussion, we decided to cover the following patterns:

  • 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

The second one should have been a bit clearer:

  • 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). If all the attachments have the fallback property, no warnings should be printed.

We'll improve this in future versions.

seratch avatar Sep 02 '22 08:09 seratch

those criteria make sense, but i'm still getting a warning despite including a fallback in the attachment

e.g. here's a more minimal reproducible test case:
  await slack.chat.postMessage({
    channel,
    attachments: [
      {
        fallback: 'fallback2',
        color: '#ff0000',
        blocks: [
          {
            'type': 'header',
            'text': {
              'type': 'plain_text',
              'text': 'header 1'
            }
          }
        ]
      }
    ]
  });
the message posts correctly in slack but i still get the following warning:
[WARN]  web-api:WebClient:0 The top-level `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. The `text` is used in places where the content cannot be rendered such as: system push notifications, assistive technology such as screen readers, etc.

note how i included a fallback in the attachment and omitted the text field from the top level

edwmurph avatar Sep 02 '22 11:09 edwmurph

Yeah, so I meant this SDK needs to be updated in a future release.

seratch avatar Sep 02 '22 11:09 seratch

oh ok sure makes sense, thanks for giving this attention

i created a PR for this to help move it forward https://github.com/slackapi/node-slack-sdk/pull/1529

edwmurph avatar Sep 02 '22 12:09 edwmurph