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.

Sign up for GitHub

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

Jump to bottom

Enhancement/slack bolt #2767

Merged
merged 45 commits into from
Jan 10, 2023
Merged

Enhancement/slack bolt #2767

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
2d92e09
Inital work
kevgliss Dec 13, 2022
7a1933a
More bug fixes
kevgliss Dec 13, 2022
85f338d
Adding list participants
kevgliss Dec 13, 2022
7eee62d
Moving error messages
kevgliss Dec 13, 2022
1c20dad
Adds exceptions
kevgliss Dec 13, 2022
34b93ea
Formatting messages
kevgliss Dec 14, 2022
d5cd4c6
Modal submission
kevgliss Dec 16, 2022
25bbda1
More experiments
kevgliss Dec 16, 2022
7471853
Making modal success messages more straight forward
kevgliss Dec 20, 2022
1bd965d
Adding a decorator module
kevgliss Dec 20, 2022
a33e1b3
Allowing unregistered users
kevgliss Dec 20, 2022
75d858d
Resolve KeyError when sending ephemeral message
wssheldon Dec 20, 2022
264d92c
Merge branch 'master' into enhancement/slack-bolt
kevgliss Dec 20, 2022
f30ef0e
Resolves unexpected keyword argument 'code' in BoltResponse
wssheldon Dec 20, 2022
4462616
Resolves async ack never awaited error
wssheldon Dec 20, 2022
2b3b7dd
Enhancement/bolt bookmarks (#2784)
wssheldon Dec 20, 2022
5bf369b
check for existence of body and view in global error handler and ack …
wssheldon Dec 22, 2022
e8990fa
resolves hash conflict in incident report view and resolves UTC event…
wssheldon Dec 22, 2022
9257334
resolves report-tactical command, update view, and pass org slug
wssheldon Dec 22, 2022
ea86e09
implements notification group update command
wssheldon Dec 23, 2022
219cfb0
implement config middleware, functionify refetch_db, and uncomment co…
wssheldon Dec 23, 2022
52ca4e8
update configuring-slack docs to include bookmarks.write scope
wssheldon Dec 23, 2022
968dca6
minor cleanup of unused args, typechecking, and duplicate strings
wssheldon Dec 23, 2022
4dc9c29
resolve workflow exceptions, blockkit bugs, and front-end UI bugs
wssheldon Dec 23, 2022
6ff69a5
Merge branch 'master' into enhancement/slack-bolt
wssheldon Dec 23, 2022
299b95c
enhancement/conversation-bookmarks (#2799)
wssheldon Jan 3, 2023
2724c19
Merge branch 'master' into enhancement/slack-bolt
wssheldon Jan 3, 2023
6eb01bd
removes moved document update code
wssheldon Jan 3, 2023
e52edbb
mvilanova review: address most comments
wssheldon Jan 5, 2023
0d21ad3
mvilanova review: resolve final comments - oncall to lazy func
wssheldon Jan 5, 2023
b1e13d5
Update src/dispatch/plugins/dispatch_slack/incident/interactive.py
wssheldon Jan 5, 2023
eb76e5d
revert oncall success message
wssheldon Jan 5, 2023
1f91b7e
format workflowinstancetab vue file
wssheldon Jan 5, 2023
1a94277
format interactive.py with black to resolve linter issue
wssheldon Jan 5, 2023
d0d26e1
HTTPStatus value from http module and fix typo in engage oncall docst…
wssheldon Jan 5, 2023
2db6247
only build modal if there's a body in error handler
wssheldon Jan 6, 2023
b166152
add type checking to bolt code and fix participant counter func
wssheldon Jan 9, 2023
25743c4
type check workflow code
wssheldon Jan 9, 2023
e6af588
Merge remote-tracking branch 'origin/master' into enhancement/slack-bolt
mvilanova Jan 9, 2023
283d471
Adds missing db commit and fixes docstring
mvilanova Jan 9, 2023
77cad1a
Adds missing filter for channel join and leave subtype messages
mvilanova Jan 9, 2023
5cf4b0c
add args back to handle_message_events func
wssheldon Jan 9, 2023
ed98ba6
reimplement member_joined_channel and other minor fixes
wssheldon Jan 10, 2023
697f783
initial work to make error handling a better ux
wssheldon Jan 10, 2023
7d6f4ac
Merge branch 'master' into enhancement/slack-bolt
kevgliss Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/dispatch/plugins/dispatch_slack/bolt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from http import HTTPStatus
import logging
from typing import Any

from blockkit import Section, Modal
from slack_bolt.app.async_app import AsyncApp
from slack_bolt.response import BoltResponse
from slack_bolt.adapter.starlette.async_handler import AsyncSlackRequestHandler
from slack_sdk.web.async_client import AsyncWebClient

from fastapi import APIRouter

Expand All @@ -28,7 +30,9 @@


@app.error
async def app_error_handler(error, client, body, logger):
async def app_error_handler(
error: Any, client: AsyncWebClient, body: dict[str, Any], logger: logging.Logger
) -> BoltResponse:
if body:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there can be a situation where an internal app error occurs that isn't related Bolt/Slack and this will not trigger (and result in a silent fail). Going to test and revisit.

Copy link
Contributor

Choose a reason for hiding this comment

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

This ended up being a a lazy listener issue: slackapi/bolt-python#635

Error handling still needs some work to mirror the old implementation though. Primarily around relaying back user friendly error messages through ephemeral responses in Slack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If possible, we should try and send error messages where the user is. e.g. if a user submits a modal and there is an error, the error should appear in the modal. Sending an ephemeral message in these cases is confusing as the user isn't quite sure which action they took resulted in an error. We can always fall back to ephemeral messages in other when it's not possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, it should be easy to check for a response_url within the global error handler to mostly get the ideal error flow:

    # the user is in a message flow
    if body.get("response_url"):
        await respond(text=str(error), response_type="ephemeral")

One issue w/ this is that Lazy Listener functions don't seem to surface exceptions within them to the error handler at all and also don't currently log the actual exception, it's just generic:

async def handle_add_timeline_submission_event(
    body: dict,
    user: DispatchUser,
    client: AsyncWebClient,
    context: AsyncBoltContext,
    form_data: dict,
):
    """Handles the add timeline submission event."""
    raise Exception("There was an error!")

ERROR:Failed to run an internal function (There was an error!):/Users/wshel/.pyenv/versions/3.9.7/envs/dispatch/lib/python3.9/site-packages/slack_bolt/lazy_listener/async_internals.py:request_wired_wrapper:30

Screenshot 2023-01-10 at 11 06 23 AM

When this happens the user will be stuck on the intermediary model created by the ack_ function and there will be no indication of an issue. Looking for potential workarounds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One issue w/ this is that Lazy Listener functions don't seem to surface exceptions within them to the error handler at all and also don't currently log the actual exception, it's just generic.

Perhaps we just handle exceptions (e.g. wrap them in try: catch) in lazy functions (instead of relying on the global handler)?

logger.info(f"Request body: {body}")

Expand All @@ -54,7 +58,7 @@ async def app_error_handler(error, client, body, logger):
configuration_middleware,
],
)
async def handle_message_events():
async def handle_message_events() -> None:
"""Container function for all message functions."""
await message_dispatcher.dispatch(**locals())

Expand Down
Loading