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

Fix #1622 Additional check for exceptions from aiohttp #1632

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

lingfish
Copy link
Contributor

Try to avoid internal exceptions from aiohttp in message_data

Summary

Fix for #1622

Testing

I couldn't figure out a way to unit test for this, and the integration tests were over my head.

Category

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs (Documents)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements

  • 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 python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

Try to avoid internal exceptions from aiohttp in `message_data`
Copy link

Thanks for the contribution! Before we can merge this, we need @lingfish to sign the Salesforce Inc. Contributor License Agreement.

@lingfish
Copy link
Contributor Author

Attempted signing of the CLA, got this:

Could not sign the CLA, please contact: oss-bots@salesforce.com

@seratch seratch closed this Jan 12, 2025
@seratch seratch reopened this Jan 12, 2025
@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 3x socket-mode area:async labels Jan 12, 2025
@seratch seratch added this to the 3.34.1 milestone Jan 12, 2025
@seratch seratch self-assigned this Jan 12, 2025
@seratch
Copy link
Member

seratch commented Jan 12, 2025

Sorry for the error. Closing and reopening this PR resolved the issue (I know this is not great but this is the workaround we're aware so far).

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

The changes look good to me; once the CI builds pass, we will merge it within a few business days

Copy link

codecov bot commented Jan 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.03%. Comparing base (86169db) to head (4a2b4c2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1632      +/-   ##
==========================================
- Coverage   85.06%   85.03%   -0.04%     
==========================================
  Files         113      113              
  Lines       12658    12658              
==========================================
- Hits        10768    10764       -4     
- Misses       1890     1894       +4     

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

@WilliamBergamin WilliamBergamin merged commit 8257c27 into slackapi:main Jan 13, 2025
13 checks passed
@seratch seratch changed the title Additional check for exceptions from aiohttp Fix #1622 Additional check for exceptions from aiohttp Jan 14, 2025
@lingfish lingfish deleted the fix-issue-1622 branch January 14, 2025 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:async bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented cla:signed socket-mode Version: 3x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants