mailjet / mailjet-gem

[API v3] Mailjet official Ruby GEM
https://dev.mailjet.com
Other
130 stars 72 forks source link

allow accessing error data #37

Closed AlexandreBernard closed 9 years ago

AlexandreBernard commented 9 years ago

I can see on the exception code that the api returns some more detailed json but - stop me if i'm wrong - we can't programmatically access it rescuing mallet exceptions as it's mixed with some other strings.

would it be possible to make it accessible?

sorry if it's not ruby specific but also it would be great to have standard codes for each errors on the api.

for example this error is not specific enough:

"{ \"ErrorInfo\" : \"\", \"ErrorMessage\" : \"MJ18 A Contact resource with value \"contact@xxxx.com\" for Email already exists.\", \"StatusCode\" : 400 }"

for now i will test for "Email already exists" presence but it's a really dirty hack. Accessing this json within exception rescuing and having something like ErrorCode: 'ExistingEmail' would be the perfect thing

Alexandre

swooop commented 9 years ago

Hi Alexandre,

Thanks for bringing this up. If I am clear, you are looking for a key/value pair to describe the error without the extra text for error handling?

You are also correct this is not Ruby specific as the API just returns JSON.

Let me check with the API team to see if there's something I've missed.

Kind regards,

Toby.

AlexandreBernard commented 9 years ago

Hi Toby,

yep that would be something perfect.

but the part that's ruby specific is that you do not let people access the raw api response while rescuing the exception.

here's my current code with the trick:

  begin
      contact = Mailjet::Contact.create(email: email, name: name)
    rescue Mailjet::ApiError => e
      if e.message.downcase.include?("email already exists")
        set optin_newsletter_synchronized: true
      else
        raise
      end
    end

here's what we should be able to do IMHO:

  begin
      contact = Mailjet::Contact.create(email: email, name: name)
    rescue Mailjet::ApiError => e
      if e.raw_response["ErrorCode"] == "ExistingEmail"
        set optin_newsletter_synchronized: true
      else
        raise
      end
    end

I think it's best to let us play with the raw response rather than creating another exception type in ruby which could become out to date and needs more work to maintain.

Alexandre

swooop commented 9 years ago

Ah, I see - I'll investigate.

Many thanks for the examples.

WeshGuillaume commented 9 years ago

Hi !

this "{ "ErrorInfo\" : "", "ErrorMessage" : "MJ18 A Contact resource with value \"contact@xxxx.com\" for Email already exists.", "StatusCode" : 400 }" is actually the raw response from the Mailjet servers. I will ask the API team if they plan to provide explicit error codes, otherwise I might consider adding a Hash that could map error data to a unique error code.

Regards, Guillaume

WeshGuillaume commented 9 years ago

Hi !

Adding a hash would actually be a bad Idea.. and probably too heavy. The API itself might send back an unique error code in the future, but it would be a long term project. At the moment, your trick is surely the best solution to get a detailed error description.

Regards, Guillaume.

WeshGuillaume commented 9 years ago

Hey @AlexandreBernard, If you have any question, i would be happy to help ! Otherwise, could you close this issue ?

Regards, Guillaume