Using callable object instances as middleware does not work.

Summary

The various methods for registering middleware in Bolt ostensibly take any Callable item. If the middleware is an instance of a callable class (i.e. a class that defines a __call__ method) rather than a function or method, and if the logging level is turned up DEBUG, then Bolt throws an exception just before it would have called the middleware.

Registering a callable object as middleware is useful for a variety of activities such as authentication handlers that need to maintain context or connections to other services.

Reproducible in:

The `slack_bolt` version

slack-bolt==1.2.1
slack-sdk==3.2.0

Python runtime version

Python 3.8.2

OS info

ProductName: Mac OS X
ProductVersion: 10.15.7
BuildVersion: 19H2
Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64

Steps to reproduce:

Create some callable class:

class MyMiddleware:
    def __call__(self, body, context, logger):
        user = context['foo'] = body["user_id"] if "user_id" in body else body["user"]["id"]
        logger.debug(f"The user is: {user}")

Turn the logging level up to DEBUG:

import logging
logging.basicConfig(level=logging.DEBUG)

Create an app that uses this middleware:

app = slack_bolt.App()
my_middleware = MyMiddleware()
app.use(my_middleware)

@app.command("/foobar")
def my_command(ack):
    ack("Hello!")

Run the Bolt app and trigger a handler

Expected result:

The Middleware object's __call__ method should be called prior to any handlers.

Actual result:

The Bolt framework throws an AttributeError exception in middleware/custom_middleware.py because, unlike a regular function or method, a callable object does not have a __name__ attribute.

Analysis

The problem is caused by debug logging in app/app.py trying to read the name property of the CustomMiddleware object, which naïvely attempts to read the __name__ attribute of the registered callable, but instances of callable classes do not have this attribute. The same bug is present in AsyncCustomMiddleware.

It is possible that a related bug exists (without needing the logging level to be turned up) if a callable object instance is used for a "lazy function", since multiple pieces of code in listener/thread_runner.py and listener/asyncio_runner.py attempt to check the lazy function __name__ attribute against request.lazy_function_name.

The solution to this is likely to introduce a new utility function to find the name of a callable, e.g.:

def callable_name(func):
    if hasattr(func, "__name__"):
        return func.__name__
    else:
        return f"{func. __class__.__module__}.{func.__class__.__name__}"

and then use this wherever we need the name of a callable provided by the user.

@nickovs

Hey @nickovs, thanks for catching this bug, providing a well thought out and detailed analysis, and suggesting an elegant solution. 👌🏻

Your suggestion appears sound to me, but I'm going to let @seratch step-in on this issue to discuss the solution before we open a pull request to solve.

@nickovs

@nickovs Thanks for sharing the thorough report of the issue. You are right about the issue described here, and the suggested solution looks great to me.

If possible, I would love to have your contribution in the commit history. Do you have the time to send a pull request? Having corresponding tests in the following files would be appreciated.

If you want me to work on the fix, I am happy to do so. Let me know your availability.

Fixed fetching of names for callable objects (slackapi#215)

I have created a fix and test cases for the fix, and submitted a Pull Request.

Note that my suspicions about a similar problem existing in the lazy function handling were correct, so I have also fixed the code there and I've added test cases for passing callable-class instances as lazy functions as well.

While the issue also in theory existed for async middleware and async lazy functions, right now there is no way to trigger the bug. This is because an instance of a class that has an async __call__ method fails the test inspect.iscoroutinefunction(), so the registration functions will not accept these objects. I've fixed the code in the async paths anyway, in case someone comes up with a more reliable test for async callables.

Fixed fetching of names for callable objects (slackapi#215)

Fixed fetching of names for callable objects (#215)

Thanks for your contribution #216 🎉 I will release a new patch version shortly.

Glad to help. Thank you for the fast turn-around!

Navigation Menu

Repository navigation

Using callable object instances as middleware does not work. #215

Summary

Reproducible in:

The `slack_bolt` version

Python runtime version

OS info

Steps to reproduce:

Expected result:

Actual result:

Analysis

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Using callable object instances as middleware does not work. #215

Description

Summary

Reproducible in:

The slack_bolt version

Python runtime version

OS info

Steps to reproduce:

Expected result:

Actual result:

Analysis

Activity

mwbrooks commented on Jan 19, 2021

seratch commented on Jan 19, 2021

nickovs commented on Jan 20, 2021

seratch commented on Jan 20, 2021

nickovs commented on Jan 20, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

The `slack_bolt` version