Skip to content

[Feature Request] Ability to shortcircuit requests by setting a response in listener middlewares #193

Description

@Ambro17

Problem

I'm currently developing an app and i have certain preconditions for each command.
In particular, each user must belong to a team for most commands, so i want to group
that logic in a listener middleware

What i've tried

So what i would like is the ability to ask for that precondition
on a common listener middleware and respond with a generic
message if the user doesn't belong to a team yet.

At first, i tried with a listener middleware similar to this

from slack_bolt import BoltResponse

def check_user_has_team(context, next):
    slack_user_id = context['slack_user_id']
    user = db.get_user(slack_user_id)
    if not user.team:
        # Shortcircuit request and respond with
        # The reason we will not continue
        return BoltResponse(status=200, body=f'User {slack_user_id} has no team')
    else:
        # If the user has a team, handle command logic
        next()


@app.command('/command_that_requires_team', middleware=[check_user_has_team])
def myhandler():
    pass

But i discovered that since the listener middleware doesn't call next(), it
is interpreted as if the command shouldn't handle the request and it ends with

BoltResponse(status=404, body={"error": "unhandled request"})

due of the logic on slack_bolt/app/app.py#351

resp, next_was_not_called = listener.run_middleware(req=req, resp=resp)
if next_was_not_called:
    # The last listener middleware didn't call next() method.
    # This means the listener is not for this incoming request.
    continue

So i tried moving it to a global middleware and it worked.
Because they are currently able to edit the response object. (Source)

resp = middleware.process(req=req, resp=resp, next=middleware_next)
if not middleware_state["next_called"]:
    if resp is None:
        return BoltResponse(
            status=404, body={"error": "no next() calls in middleware"}
        )
    return resp

The problem with this approach is that this check is not global.
Some commands can be used without a team, but most of them don't.

Feature Request

So my feature request would be to add the same possibility for listener
middlewares, so we can rely on them to group common logic.
I slightly edited bolt's source code and i managed to get the expected behaviour,
but i don't know the codebase enough to judge if it's a safe change or if it's
backward compatible (i believe it wouldn't be) but feel free to correct me

resp, next_was_not_called = listener.run_middleware(req=req, resp=resp)
if next_was_not_called:
+    if resp is not None:
+        # The listener middleware decided that this request
+        # Should end by setting the response object
+        return resp

    # The last listener middleware didn't call next() method.
+    # And didn't set the response object
    # This means the listener is not for this incoming request.
    continue

Plus (+) denotes added lines, there's no other modified/deleted lines

Alternative #1: Listener middlewares that can set the response

One way to workaround this is by using function decorators and decorate each
function to check if a team exists, but i think the functionality between
this decorators and middlewares may overlap.
Personally, i think the middleware api is a bit more intutive
(specially for beginners) and composable, hence the feature request.

But perhaps i missed some edge case on why we can't do this, feel free to correct
me if that's the case

Alternative #2: Move to a global middleware with a command filter

Add a conditional on the global middleware to ignore the check based on commands.
I don't like this approach because it is high-maintenance and the check is not close
to the handler logic, as it would be with the middleware and decorator approach.

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

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Others

Thanks in advance for reading the wall of text!

Activity

  1. added this to the 1.2.0 milestone on Jan 5, 2021
  2. seratch commented on Jan 5, 2021

    @seratch
    Contributor

    Thanks for the feedback (as always 😃 ). I agree this needs to be improved. Your diff already looks good to me but I will check if the implementation works for any cases.

  3. modified the milestones: 1.2.0, 1.1.5 on Jan 5, 2021
  4. seratch commented on Jan 6, 2021

    @seratch
    Contributor

    Thanks for the feedback! I just sent a pull request to fix this issue #194

  5. Ambro17 commented on Jan 6, 2021

    @Ambro17
    Author

    Wow, That was fast! Thank you @seratch

  6. added a commit that references this issue on Jan 7, 2021
    55d4f16
  7. gfang-work commented on Aug 22, 2022

    @gfang-work

    @seratch - Thanks for the work. I followed this and it looks successful. But I got the "This didn't work, try again." error pop up in the bottom right corner of the Slack UI. Do you have any pointers to resolve this?

  8. seratch commented on Aug 22, 2022

    @seratch
    Contributor

    @gfang-work As long as your app responds with 200 OK HTTP response, the error never arises.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions