Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interactive-messages: log payload object when handler not found #928

Closed
5 of 15 tasks
davidalpert opened this issue Dec 10, 2019 · 3 comments
Closed
5 of 15 tasks

interactive-messages: log payload object when handler not found #928

davidalpert opened this issue Dec 10, 2019 · 3 comments
Assignees
Labels
enhancement M-T: A feature request for new functionality pkg:interactive-messages (deprecated) applies to `@slack/interactive-messages`

Comments

@davidalpert
Copy link

Description

The callback registration pattern is a powerful way to decouple the concerns of handling slack interactive-message callbacks, separating request parsing and validation logic common to all slack app callbacks from the business logic of handling and responding to those callbacks.

One thing I struggled with when moving from handling the raw requests myself to using the @slack/interactive-messages@1.4.0 package was losing access to the raw request or underlying payload. Once I had a handler matching I could see the payload inside my handler to learn how to better handle that type of payload. When I didn't have any matching handler, however, I couldn't understand why my matchers were failing to match.

In these cases when no handler is found it would be very useful to expose the raw payload in the interactive-messages adapter debug logs.

This could be as simple as adding the following:

debug({ payload })

to the adapter.dispatch(...) method just after logging that "dispatch could not find a handler"

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Packages:

Select all that apply:

  • @slack/web-api
  • @slack/events-api
  • @slack/interactive-messages
  • @slack/rtm-api
  • @slack/webhooks
  • I don't know

Reproducible in:

package version:

  • @slack/interactive-messages@1.4.0

node version:

  • v10.16.0

OS version(s):

  • macOS Catalina 10.15.1

Steps to reproduce:

  1. register a callback with constraints that do not match the callback (e.g. misspell the callback_id value)
  2. enable debug logging in your app
  3. initiate the callback from slack
  4. see the dispatch could not find a handler message in the logs

Expected result:

See the raw { payload } object in the logs so that I can learn what slack sends and compare it to my registered matchers to understand why none of my handlers matched.

Actual result:

Cannot see the inputs (the payload) so cannot understand why my configured callback handlers didn't match and fire.

@stevengill stevengill added the pkg:interactive-messages (deprecated) applies to `@slack/interactive-messages` label Dec 10, 2019
@stevengill
Copy link
Member

Hey @davidalpert,

I personally like your suggestion. It will add a little noise to the console output but I see the benefit of it. I'll check with the rest of the team to see if they are okay with it before proceeding.

@stevengill stevengill added the enhancement M-T: A feature request for new functionality label Dec 10, 2019
@davidalpert
Copy link
Author

thank you. even if only logged by the adapter on those callbacks which fail to get matched/routed it would help 👍

stevengill added a commit to stevengill/node-slack-sdk that referenced this issue Dec 11, 2019
@stevengill stevengill self-assigned this Dec 11, 2019
stevengill added a commit that referenced this issue Dec 12, 2019
added payload to debug when no handler matches. #928
@stevengill
Copy link
Member

Merged in the fix for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality pkg:interactive-messages (deprecated) applies to `@slack/interactive-messages`
Projects
None yet
Development

No branches or pull requests

2 participants