feat: surface ack_timeout on the handler

[WilliamBergamin]

Summary

These changes aim to surface ack_timeout at the function handler level. This allows developers to set their own custom timeout for the acknowledgement, this comes in handler when dealing with dynamic options or new function types that may allow for a timeout greater then 3 seconds

Testing

Set up an app with the following function handler

@app.function("search", auto_acknowledge=False, ack_timeout=5)
def handle_search_step_event(
    ack: Ack, inputs: dict, fail: Fail, complete: Complete, logger: logging.Logger
):
    logger.info("Function inputs received:")
    logger.info(f"inputs: {json.dumps(inputs, indent=2)}")
    sleep(4)
    complete(outputs={...})

Bolt will allow your handler to take up to 5 seconds before returning an error response to Slack

Configuring the handler with auto_acknowledge=true (default behavior)

@app.function("search", ack_timeout=5)
def handle_search_step_event(
    ack: Ack, inputs: dict, fail: Fail, complete: Complete, logger: logging.Logger
):
    logger.info("Function inputs received:")
    logger.info(f"inputs: {json.dumps(inputs, indent=2)}")
    ...

Will print a warning indicating that ack_timeout has no effect

WARNING:slack_bolt.App:As you gave `client` as well, `token` will be unused.
WARNING:slack_bolt.App:On @app.function("search"), as `auto_acknowledge` is `True`, the `acknowledgement_timeout=5` you gave will be unused
INFO:slack_bolt.App:A new session has been established (session id: a7bd6667-e9db-45dc-8419-468afbf5e5ea)

Feedback

Instead of naming this acknowledgement_timeout should e name it ack_timeout? 🟢

Category

• 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

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.

[WilliamBergamin]

Should we name this ack_timeout rather then acknowledgement_timeout?

[zimeg]

thought: ack_timeout makes sense to me and matches the ack parameter well! 📚 ✨

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.96%. Comparing base (76f7027) to head (7f440d7).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1351      +/-   ##
==========================================
- Coverage   90.96%   90.96%   -0.01%     
==========================================
  Files         222      222              
  Lines        7507     7517      +10     
==========================================
+ Hits         6829     6838       +9     
- Misses        678      679       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zimeg]

@WilliamBergamin LGTM! Awaited ack seems so good for tasks needing time 👾 ✨

I left a few comments of thoughts and idea but nothing blocking. Your suggestion of ack_timeout is solid IMHO 👀

[zimeg]

thought: ack_timeout makes sense to me and matches the ack parameter well! 📚 ✨

[zimeg]

quibble: Hardcoding the expected outputs might make this output more immediate at a glance! I'm not so familiar with kwargs I must admit 😔

[WilliamBergamin]

I used kwargs in the test in order to assert that the parameter name shown in the warning is the one defined by the function 🤔 I think we should leave it for now but we don't need to make this a common pattern

[zimeg]

@WilliamBergamin LGTM and I'm so optimistic in the change to match ack around- 👾 ✨