slackapi / bolt-python

A framework to build Slack apps using Python
https://slack.dev/bolt-python/
MIT License
1.04k stars 238 forks source link

Global middleware example logic error #1110

Closed curious-broccoli closed 1 month ago

curious-broccoli commented 1 month ago

The page URLs

Issue

In the current example I think the function is always executed till the end and next is always called, which means the event callback will be called even if authentication failed.

@app.use
def auth_acme(client, context, logger, payload, next):
    slack_user_id = payload["user"]
    help_channel_id = "C12345"

    try:
        # Look up user in external system using their Slack user ID
        user = acme.lookup_by_id(slack_user_id)
        # Add that to context
        context["user"] = user
    except Exception:
        client.chat_postEphemeral(
            channel=payload["channel"],
            user=slack_user_id,
            text=f"Sorry <@{slack_user_id}>, you aren't registered in Acme or there was an error with authentication. Please post in <#{help_channel_id}> for assistance"
        )

    # Pass control to the next middleware
    next()

Change

Similar to bolt-js example it should probably return if authentication fails and not proceed to the next middleware.

WilliamBergamin commented 1 month ago

Hi @curious-broccoli thanks for writing in 💯

Bolt-js and Bolt-python operate slightly differently. Calling the next() method allows the following middleware and the handler to execute, this ensures that a response is sent for each request.

If a global middleware raises an Exception or returns early then no response is sent for the request, this may lead to unexpected behaviors like retries and errors. In most cases the handler should respond appropriately to the request based on the data collected by the middleware. This is why next() is shown in our documentation.

Depending on what an app is trying to accomplish it may be valid to return early in a middleware but this may lead to unexpected behaviors.