Bounce Code Type Issue

[travelton]

In the Bounce struct of bounces.go, the Mailgun API returns a Code of type Int or String.

To duplicate, issue gun.GetBounces(-1, -1). If any bounces in the first page of 100 results contain a string and int in the Bounce, you'll get a type error when the json library unmarshalls the Code result to the Bounce struct.

"Error: json: cannot unmarshal string into Go value of type int"

[sam-falvo]

I know I'm a bit late for this, but isn't this technically a bug in the Mailgun API code?

[mbanzon]

I think so. The code in question is a SMTP reply code (and the RFC states that they are three digits) and should only be an integer.

The question remains if this is better to solve in this library or a fix in Mailgun API is a possibility?

[johnnylee]

Is there a workaround for this? I'm trying to figure out how to deal with bounces in Go.

[sam-falvo]

I thought a fix for this issue was already applied. :( I'm hoping the issue was left open by accident. Stand by.

[sam-falvo]

I'm trying to submit a PR for it now. Hang tight.

[EtienneBruines]

An SMTP reply consists of a three digit number (transmitted as
three alphanumeric characters)

• The RFC

This means that they should be transmitted as characters (not one integer), but they should by no means be quoted (because then it wouldn't consist of three digit numbers). I feel like this should be fixed in the Mailgun API, as it's their code that's faulty, and not the Go code that should cope. Thoughts @mbanzon, @sam-falvo ?

[mbanzon]

As I understood it changing this on the Mailgun side would introcude a breaking change in the API. As I see it the only option would be to have a workaround in the library (perhaps with a helper function - for those places/applications that require an actual integer response code).

[sam-falvo]

I have a PR that has so far gone unreviewed; thinking of just committing it. It does break the API of the mailgun-go library though, because it turns that field from an integer to an interface{}. In exchange for this, though, it introduces a getter function which does return a proper integer along with an error.

See https://github.com/mailgun/mailgun-go/pull/38/files#diff-ceabd71e327239278abe0bc6cd261565R39

If someone can please review and let me know if this will meet your needs, I can just merge this myself without Mailgun's explicit OK, as this issue seems to be very low priority. Thanks! @EtienneBruines @mbanzon

[EtienneBruines]

LGTM. Perhaps the testing-function should also check on parsed JSON? (for the weird (and possibly non-existing) scenario that the type doesn't get resolved correctly? )

[mbanzon]

lgtm

[sam-falvo]

I believe that's the last test case (where we expect a syntax error). I'll commit the change, but if the test can be improved, let me know. :) Thanks!