Skip to content

Raise TypeError instead of a str in to_bytes and to_str - #1229

Closed
CasperGN wants to merge 2 commits into
dapr:mainfrom
CasperGN:fix/to-bytes-raise
Closed

CasperGN wants to merge 2 commits into
dapr:mainfrom
CasperGN:fix/to-bytes-raise

Conversation

@CasperGN

Copy link
Copy Markdown
Contributor

Description

to_bytes and to_str in dapr/clients/grpc/_helpers.py ended with raise f'invalid data type {type(data)}'. Raising a string is itself an error in Python, so a caller who passed the wrong type got TypeError: exceptions must derive from BaseException and never saw the real message. Both now raise TypeError(f'invalid data type {type(data)}'). Callers already got a TypeError, so anyone catching it is unaffected; they just get the right message now. Added unit tests for both helpers. A repo-wide search found no other raise of a string.

RELEASE NOTE: FIX to_bytes and to_str now raise a TypeError with a clear message when given a value that is neither str nor bytes.

Issue reference

Please reference the issue this PR will close: N/A, found while reviewing #1228, no existing issue filed.

Checklist

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

🤖 Generated with Claude Code

Both helpers ended with `raise f'invalid data type ...'`. Raising a str is
itself an error, so callers got "TypeError: exceptions must derive from
BaseException" and the intended message was lost. Raise a TypeError with
the message, which keeps the exception class callers already see.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
@CasperGN
CasperGN requested review from a team as code owners September 24, 2026 10:22
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.91%. Comparing base (fb229bc) to head (3544a83).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1229      +/-   ##
==========================================
+ Coverage   83.89%   83.91%   +0.01%     
==========================================
  Files         123      123              
  Lines       10265    10265              
==========================================
+ Hits         8612     8614       +2     
+ Misses       1653     1651       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sicoyle sicoyle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you!

@sicoyle
sicoyle added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 24, 2026
@sicoyle
sicoyle added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 24, 2026
@sicoyle
sicoyle added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 24, 2026
@CasperGN

Copy link
Copy Markdown
Contributor Author

Closing in favor of #1220 that came first

@CasperGN CasperGN closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants