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

Add support for direct responses #1

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

roborourke
Copy link

@roborourke roborourke commented Mar 29, 2017

Sometimes in the case of slash commands it's useful to have the users command displayed too - it helps to educate / show other users how to use a command they may not have seen before.

It can also add context to a response rather than the response having to indicate what just happened when it's visible to all users.

Without a direct response

What I see:

Me: /do-thing
Bot: Thing done!

What everyone else sees:

Bot: Thing done!

With a direct response

Everyone sees:

Me: /do-thing
Bot: Thing done!

Currently all responses are delayed using thee response URL for slash commands, this allows for direct responses to the original request.
callback() is triggered automatically if no error is passed when the node event loop is empty. No need for any fancy handling.
Sometimes with lambda you're hitting a still running instance of the function so the binding happens on each call causing the callback to be prepended to the args multiple times.
The way you have to send URL encoded complex messages to slack means we need some custom serialisation. Values like the attachments array will be stripped by `qs.stringify()` so we need to `JSON.stringify()` those values and then URL encode them. 🤦‍♂️
AWS keeps the same function alive and in memory but the handler is called each time. This updates the callback and exposes it during handler execution.
@roborourke
Copy link
Author

@johnagan this is pretty well tested now in my current use cases, let me know if you have a better solution in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant