Skip to content

fix: Improve type annotations in SignatureVerifier#1845

Merged
WilliamBergamin merged 2 commits intoslackapi:mainfrom
BHSPitMonkey:patch-1
Mar 25, 2026
Merged

fix: Improve type annotations in SignatureVerifier#1845
WilliamBergamin merged 2 commits intoslackapi:mainfrom
BHSPitMonkey:patch-1

Conversation

@BHSPitMonkey
Copy link
Contributor

Summary

Receive headers as abstract/immutable collections.abc.Mapping instead of Dict to more accurately declare compatibility with dict-like objects (common for holding headers in HTTP frameworks), and mark timestamp/signature args as Optional in is_valid to reflect reality and fix the typechecker errors in is_valid_request.

Testing

These changes only touch type annotations which have no runtime effects; The only expectation here is that type checks pass.

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.

Receive headers as abstract/immutable `collections.abc.Mapping` instead of `Dict` to more accurately declare compatibility with dict-like objects (common for holding headers in HTTP frameworks), and mark timestamp/signature args as `Optional` in `is_valid` to reflect reality and fix the typechecker errors in `is_valid_request`.
@BHSPitMonkey BHSPitMonkey requested a review from a team as a code owner March 24, 2026 18:46
@salesforce-cla
Copy link

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

@codecov
Copy link

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.99%. Comparing base (0cb9728) to head (bdb6cac).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1845   +/-   ##
=======================================
  Coverage   83.99%   83.99%           
=======================================
  Files         117      117           
  Lines       13252    13253    +1     
=======================================
+ Hits        11131    11132    +1     
  Misses       2121     2121           

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

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

Hi @BHSPitMonkey thanks for opening this PR 🙏

Love seeing those # type: ignore go away!
Changes seem solid but aren't compatible with python 3.7 and 3.8, I left a comment on how to potentially address it, once resolved we should be good to merge

self,
body: Union[str, bytes],
headers: Dict[str, str],
headers: Mapping[str, str],
Copy link
Contributor

Choose a reason for hiding this comment

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

For python 3.7 and 3.8 this Mapping type is not importable from collections.abc causing

slack_sdk/signature/__init__.py:30: in SignatureVerifier
    headers: Mapping[str, str],
E   TypeError: 'ABCMeta' object is not subscriptable

Importing Mapping as follows should fix the issue 🤔

from typing import TYPE_CHECKING,  Dict, Optional, Union

if TYPE_CHECKING:
    from collections.abc import Mapping
else:
    Mapping = Dict

Copy link
Contributor

Choose a reason for hiding this comment

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

If we go this route, it might be a good idea to also include a comment about how this is for python 3.7 and 3.8 backwards compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, old Pythons strike again! But I like your proposed solution - Will push it with a comment shortly.

Because `Mapping` is not subscriptable until Python 3.9, the previous change introduced runtime errors for 3.7/3.8. Applies suggested workaround from @WilliamBergamin to continue using Dict at runtime until <3.9 is eventually dropped.
from typing import Dict, Optional, Union
from typing import Dict, Optional, Union, TYPE_CHECKING

# Fallback to Dict for Python 3.7/3.8 compatibility (safe to remove once these versions are no longer supported)
Copy link
Contributor

Choose a reason for hiding this comment

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

🥇

@WilliamBergamin WilliamBergamin merged commit 4547d73 into slackapi:main Mar 25, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants