{"action":"created","issue":{"url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/131","repository_url":"https://api.github.com/repos/thriftrw/thriftrw-python","labels_url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/131/labels{/name}","comments_url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/131/comments","events_url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/131/events","html_url":"https://github.com/thriftrw/thriftrw-python/issues/131","id":176437002,"number":131,"title":"Make OverflowError messages more informative","user":{"login":"TheJakeSchmidt","id":10720216,"avatar_url":"https://avatars.githubusercontent.com/u/10720216?v=3","gravatar_id":"","url":"https://api.github.com/users/TheJakeSchmidt","html_url":"https://github.com/TheJakeSchmidt","followers_url":"https://api.github.com/users/TheJakeSchmidt/followers","following_url":"https://api.github.com/users/TheJakeSchmidt/following{/other_user}","gists_url":"https://api.github.com/users/TheJakeSchmidt/gists{/gist_id}","starred_url":"https://api.github.com/users/TheJakeSchmidt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TheJakeSchmidt/subscriptions","organizations_url":"https://api.github.com/users/TheJakeSchmidt/orgs","repos_url":"https://api.github.com/users/TheJakeSchmidt/repos","events_url":"https://api.github.com/users/TheJakeSchmidt/events{/privacy}","received_events_url":"https://api.github.com/users/TheJakeSchmidt/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-09-12T17:38:11Z","updated_at":"2016-09-12T21:10:40Z","closed_at":null,"body":"When thriftrw-python attempts to serialize a struct with a primitive field whose value is too big (for example, an i32 whose value is 2^33), the exception that bubbles up is the OverflowError from PrimitiveTypeSpec.to_wire(), which has only the message \"value too large to convert to int\". This made for a difficult debugging session when we hit this problem recently, as it took us a while to figure out which field was overflowing. It would help greatly if StructTypeSpec.to_wire() caught OverflowError (or Exception) and wrapped it with something like \"Error serializing field 2 of BestStructEver: \" + e.message, so that serialization error messages had the full path of the bad field embedded."},"comment":{"url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/comments/246494732","html_url":"https://github.com/thriftrw/thriftrw-python/issues/131#issuecomment-246494732","issue_url":"https://api.github.com/repos/thriftrw/thriftrw-python/issues/131","id":246494732,"user":{"login":"abhinav","id":41730,"avatar_url":"https://avatars.githubusercontent.com/u/41730?v=3","gravatar_id":"","url":"https://api.github.com/users/abhinav","html_url":"https://github.com/abhinav","followers_url":"https://api.github.com/users/abhinav/followers","following_url":"https://api.github.com/users/abhinav/following{/other_user}","gists_url":"https://api.github.com/users/abhinav/gists{/gist_id}","starred_url":"https://api.github.com/users/abhinav/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abhinav/subscriptions","organizations_url":"https://api.github.com/users/abhinav/orgs","repos_url":"https://api.github.com/users/abhinav/repos","events_url":"https://api.github.com/users/abhinav/events{/privacy}","received_events_url":"https://api.github.com/users/abhinav/received_events","type":"User","site_admin":false},"created_at":"2016-09-12T21:10:40Z","updated_at":"2016-09-12T21:10:40Z","body":"Hey, yes this is a good idea. We can do one of the following,\r\n\r\n- Modify `validate()` for `i{8,16,32,64}` `TypeSpec`s to raise `ValueError` if the value is out of bounds, and `validate()` for `StructSpec` to wrap `{Value,Type}Error`s to include the field name.\r\n- Modify `StructSpec.to_wire()` to wrap `OverflowError` and include the field name in the message.\r\n\r\nI'm partial to the first solution because it doesn't specialize behavior to `OverflowError` and also doesn't add a catch-all-exceptions behavior. It seems generally better to make this part of the `validate()` step. Also, since raising `{Value,Type}Error` is part of the contract of `validate()` (see [docs](https://thriftrw.readthedocs.io/en/latest/api.html#thriftrw.spec.TypeSpec.validate)) so we're not going too wide or too specific with the exceptions."}}
{"actor":{"display_login":"abhinav"}}