Skip to content

Fix #193 by enabling listener middleware to return a response - #194

Merged
seratch merged 2 commits into
slackapi:mainfrom
seratch:issue-193-middleware-listener-response
Jan 7, 2021
Merged

seratch merged 2 commits into
slackapi:mainfrom
seratch:issue-193-middleware-listener-response

Conversation

@seratch

@seratch seratch commented Jan 6, 2021

Copy link
Copy Markdown
Contributor

This pull request resolves #193 by enabling listener middleware's behavior to return a response without running its listener function.

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
  • Document pages under /docs
  • Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@seratch seratch added bug Something isn't working area:async area:sync labels Jan 6, 2021
@seratch seratch added this to the 1.1.5 milestone Jan 6, 2021
@seratch seratch self-assigned this Jan 6, 2021
@codecov

codecov Bot commented Jan 6, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #194 (fb93fe4) into main (03fdd85) will decrease coverage by 0.05%.
The diff coverage is 93.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
- Coverage   92.15%   92.10%   -0.06%     
==========================================
  Files         149      149              
  Lines        4448     4470      +22     
==========================================
+ Hits         4099     4117      +18     
- Misses        349      353       +4     
Impacted Files Coverage Δ
slack_bolt/listener/async_listener.py 98.41% <ø> (ø)
slack_bolt/workflows/step/step_middleware.py 96.66% <ø> (ø)
slack_bolt/app/app.py 86.75% <90.00%> (-0.22%) ⬇️
slack_bolt/app/async_app.py 94.37% <90.00%> (-0.46%) ⬇️
slack_bolt/listener/asyncio_runner.py 84.61% <100.00%> (ø)
slack_bolt/listener/listener.py 96.77% <100.00%> (ø)
slack_bolt/listener/thread_runner.py 89.62% <100.00%> (ø)
slack_bolt/logger/messages.py 89.36% <100.00%> (+0.98%) ⬆️
slack_bolt/middleware/async_middleware.py 90.90% <100.00%> (ø)
slack_bolt/middleware/middleware.py 90.90% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03fdd85...fb93fe4. Read the comment docs.

if next_was_not_called:
if middleware_resp is not None:
if self._framework_logger.level <= logging.DEBUG:
debug_message = debug_return_listener_middleware_response(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example log message:

DEBUG    slack_bolt.App:app.py:366 Responding with listener middleware's response - listener: handle, status: 200, body: listener middleware (3 millis)

:param req: An incoming request from Slack.
:return: The response generated by this Bolt app.
"""
starting_time = time.time()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug logs used to track only the time spent in a listener function. Starting here is more accurate.

Comment thread slack_bolt/app/app.py
# This means the listener is not for this incoming request.
continue

if middleware_resp is not None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the case when a listener middleware sets the response but also calls next, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it is 👍

@Ambro17 Ambro17 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

@seratch
seratch merged commit 4f5cd1e into slackapi:main Jan 7, 2021
@seratch
seratch deleted the issue-193-middleware-listener-response branch January 7, 2021 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:async area:sync bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants