intercom / intercom-go

Go Bindings For Intercom
https://developers.intercom.io/reference
Other
71 stars 80 forks source link

Inconsistent return types #38

Closed f2prateek closed 8 years ago

f2prateek commented 8 years ago

For some reason the intercom API returns a number instead of a string for the social profile ID. This causes the json unmarshalling code to fail with json: cannot unmarshal number into Go value of type string. (I've omitted other parts of the JSON, but the ID is real so you can look up the profile in your database).

social_profiles": [
  {
    "type":"social_profile",
    "name":"Twitter",
    "id":217893617,
...

I'm not entirely sure if this should be fixed in this library (possibly by declaring the type as json.Number) or upstream in your servers/database (I can submit a fix if you choose the former).

f2prateek commented 8 years ago

This also happens when I'm fetching companies, although I haven't yet dug into which exact field breaks the type definition.

json: cannot unmarshal number 1406353964000 into Go value of type int32

f2prateek commented 8 years ago

Another mismatch:

JSON return by the API:

"remote_created_at":1420520395000,"created_at":1429802511,"updated_at":1453153823,"

Error:

json: cannot unmarshal number 1420520395000 into Go value of type int32
josler commented 8 years ago

Hi @f2prateek

If you check out the v2/ branch, I've moved things to int64 there which helps greatly. v2 is undergoing work though, so please vendor appropriately :)

May I ask what your last snippet is from? I'm surprised that standard company fields have that level of timestamp precision?

If you contact me through Intercom (just message in asking for me) then I'll take a look.

f2prateek commented 8 years ago

@josler thanks! I can try out the v2 branch for the the fix, and I'll send over a reproducible case which causes the error as well.

I don't think the first issue (social profile id) will be fixed by that though, since the go library expects a string but the server is returning a number. I'm working around it internally by redeclaring the type to be json.Number.

josler commented 8 years ago

Yep, this (and the other couple issues) I should get a chance to take a look at this week and implement on the v2/ branch.

josler commented 8 years ago

@f2prateek

I looked into this, and it appears that the data being sent to us to set that field is:

Wed, 16 Mar 47988 14:16:40 UTC +00:00

Which is probably due to sending the wrong timestamp format to us in the first place (ms-resolution, not second-resolution which we support).

The v2/ branch does handle this, using int64, but doesn't make any attempt to parse that into a valid time.

f2prateek commented 8 years ago

Awesome, yup we're using the v2 branch and the only change I've had to make was the type of SocialProfile.ID to json.Number instead of string.