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

Middleware stacking and recursive initialization #2002

Closed
Kludex opened this issue Jan 11, 2023 Discussed in #1161 · 2 comments · Fixed by #2017
Closed

Middleware stacking and recursive initialization #2002

Kludex opened this issue Jan 11, 2023 Discussed in #1161 · 2 comments · Fixed by #2017
Labels
clean up Refinement to existing functionality

Comments

@Kludex
Copy link
Sponsor Member

Kludex commented Jan 11, 2023

Discussed in #1161

Originally posted by euri10 April 1, 2021
long story short I was writing a logging middleware whose handlers are initialized in the __init__ of the middleware and ended up with multiple logs.

it seems to me the __init__ of the middlewarea are recursively called because of the build_middleware_stack method and I'm not sure if this is intended or a bug.

here's a test case that shows what I think is an issue:

class SimpleInitializableMiddleware:
    def __init__(self, app, counter: int):
        self.app = app
        self.counter = counter
        print(self.counter)


def test_middleware_stack_init():
    app = Starlette()
    app.add_middleware(SimpleInitializableMiddleware, counter=1)
    app.add_middleware(SimpleInitializableMiddleware, counter=2)
    app.add_middleware(SimpleInitializableMiddleware, counter=3)
    app.add_middleware(SimpleInitializableMiddleware, counter=4)

I would expect to see 1 2 3 4 as the output.
however it prints 1 1 2 1 2 3 1 2 3 4 because for each add_middlware call it goes down the stack and reinitialize it.

what you think @tomchristie ?

@Kludex Kludex added the clean up Refinement to existing functionality label Jan 11, 2023
@Kludex
Copy link
Sponsor Member Author

Kludex commented Jan 11, 2023

A possible solution would be to deprecate, and further remove add_middleware().

adriangb added a commit to adriangb/starlette that referenced this issue Jan 23, 2023
@Kludex
Copy link
Sponsor Member Author

Kludex commented Feb 4, 2023

This will be available on Starlette 0.24.0.

@Kludex Kludex added this to the Version 0.24.0 milestone Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean up Refinement to existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant