egg-mode-rs / egg-mode

a twitter api crate for rust
https://crates.io/crates/egg-mode
Mozilla Public License 2.0
371 stars 65 forks source link

Allow bounding_box to be null #110

Closed travisbrown closed 3 years ago

travisbrown commented 3 years ago

I've run into a couple of cases where place decoding fails because bounding_box is null. If you currently look up user 1462977332, for example, the result includes this object:

        "place": {
          "id": "1022ca6e8a555000",
          "url": "https://api.twitter.com/1.1/geo/id/1022ca6e8a555000.json",
          "place_type": "poi",
          "name": "[Place name removed]",
          "full_name": "[Place name removed]",
          "country_code": "",
          "country": "",
          "contained_within": [],
          "bounding_box": null,
          "attributes": {}
        }

This seems to happen specifically for retweets of promoted tweets associated with the place "[Place name removed]", but I haven't really looked too closely at what's going on in these cases.

The decoding failure error message looks like this:

DeserializeError(Error("data did not match any variant of untagged enum SerEnum", line: 1, column: 6029))

When I used RoundTrip on that specific instance I got the following:

there was an error: invalid type: map, expected a boolean

Which isn't the actual problem, but the actual problem wasn't too hard to find.

The change in this PR decodes a null bounding box into an empty vector, and encodes an empty vector into null. An alternative solution would be to change the bounding_box field on Place to be optional, but the empty vector approach seemed more in line with the way Twitter is representing missingness here (e.g. the name field is the string "[Place name removed]"). This approach also shouldn't collide with any other cases where the vector is "naturally" empty, since currently the serialization assumes that it's non-empty (in that it checks whether the vector has one element, calling it a point if it does and a polygon otherwise).

I've confirmed that this change fixes the issues I was seeing. I wasn't sure what to do about tests for this kind of thing, but am happy to add some. Thanks for the library!

adwhit commented 3 years ago

Thanks, LGTM