Skip to content

Commit

Permalink
Make error clearer when assigning non-string to string field
Browse files Browse the repository at this point in the history
Summary:
Context
==

When working with some "convert this object to a Thrift object" code, a mistake in the source object was sending what should be a string field as an integer. The only error raised was
```
    1) TypeError: bad argument type for built-in operation

      File "thrift.python/types.pyx", line 616, in thrift.python.types.Struct.__init__

      File "thrift.python/types.pyx", line 130, in thrift.python.types.StringTypeInfo.to_internal_data
```

This error is about as helpful as a chocolate teapot on an open fire, to me.

This change catches the TypeError that's raised by the UTF8String call, and reframes it with context that might help an end user understand precisely what has happened. It would be even better if it would indicate which field is deemed a string in the Thrift spec, and is being passed as not-string to the code, but I don't know how to get at that data in this file.

Implementation
==
* Catch and re-raise TypeError with context details
* Add a unit test that validates the re-raise message

Reviewed By: aristidisp

Differential Revision: D53225702

fbshipit-source-id: e926d5af4eabee61f7548460c87843204b963c6e
  • Loading branch information
bajanduncan authored and facebook-github-bot committed Feb 2, 2024
1 parent f67df68 commit f55d02f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
7 changes: 7 additions & 0 deletions thrift/lib/python/test/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,13 @@ def test_init_with_invalid_field(self) -> None:
# pyre-ignore[28]: for test
easy(val=1, an_int=Integers(small=300), name="foo", val_lists=[1, 2, 3, 4])

def test_init_with_invalid_field_value(self) -> None:
with self.assertRaisesRegex(
TypeError, "Cannot create internal string data representation"
):
# pyre-ignore[6]: name is string, but under test
easy(val=1, an_int=Integers(small=300), name=1)

def test_iterate(self) -> None:
x = Reserved(
from_="hello",
Expand Down
8 changes: 7 additions & 1 deletion thrift/lib/python/types.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,13 @@ cdef class StringTypeInfo:

# validate and convert to format serializer may understand
def to_internal_data(self, object value not None):
return PyUnicode_AsUTF8String(value)
try:
return PyUnicode_AsUTF8String(value)
except TypeError as e:
raise TypeError(
"Cannot create internal string data representation. "
f"Expected type <class 'str'>, got: {type(value)}."
) from e

# convert deserialized data to user format
def to_python_value(self, object value):
Expand Down

0 comments on commit f55d02f

Please sign in to comment.