Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose RequestError in GoogleApiException when available #615

Merged
merged 3 commits into from Oct 20, 2015
Merged

Expose RequestError in GoogleApiException when available #615

merged 3 commits into from Oct 20, 2015

Conversation

saguiitay
Copy link
Contributor

This should resolve #480

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Oct 16, 2015
@saguiitay
Copy link
Contributor Author

I signed it!

@peleyal
Copy link
Collaborator

peleyal commented Oct 19, 2015

I'm just curious to know, does it work for you now? I feel that we are missing the JSON tags for all the fields. So for example, I would expect the following property
https://github.com/google/google-api-dotnet-client/blob/9618f6846b29d4b812ba61362215a2c9d25110a9/Src/GoogleApis.Core/Apis/Requests/SingleError.cs#L27
to have [Newtonsoft.Json.JsonPropertyAttribute("domain")] attribute.

Similar to: https://github.com/google/google-api-dotnet-client/blob/9618f6846b29d4b812ba61362215a2c9d25110a9/Src/GoogleApis.Auth/OAuth2/Responses/TokenResponse.cs

Does that make sense?
Thanks!
Eyal

@saguiitay
Copy link
Contributor Author

I didn't have to touch the deserialization works, since the Error object was already deserialized and used, even before my change (the message of the exception is computed by calling error.ToString())

I'll remove the extra CTRO, and just pass the Error object via the property - it'll make the code cleaner and simpler.

There appears to be 3 tests that fail, but those are not related to the Exception - they are all in the RequestBuilderTest

@@ -44,6 +44,9 @@ public GoogleApiException(string serviceName, string message, Exception inner)
/// <summary>Creates an API Service exception.</summary>
public GoogleApiException(string serviceName, string message) : this(serviceName, message, null) { }

/// <summary>The Error which was returned from the server, or null if unavailable.</summary>

This comment was marked as spam.

@peleyal
Copy link
Collaborator

peleyal commented Oct 20, 2015

Are you saying that our tests fail head? I'll need to check that out...

Otherwise your change seems ok, just make sure you address my comment and we are ready to go. (check also that you signed the CLA, since this pull request is labeled as cla:no.
What is the email address you used to sign?

Thanks!
Eyal

@peleyal peleyal added this to the 1.9.4 milestone Oct 20, 2015
Wrap null with code section in comment
@saguiitay
Copy link
Contributor Author

I fixed the comment.

I've also already signed the CLA. My Google Account uses a different email than my GitHub account, which might have caused an issue...

@saguiitay
Copy link
Contributor Author

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Oct 20, 2015
peleyal added a commit that referenced this pull request Oct 20, 2015
Expose RequestError in GoogleApiException when available
@peleyal peleyal merged commit af8f562 into googleapis:master Oct 20, 2015
@peleyal peleyal modified the milestones: 1.10, 1.9.4 Oct 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return "error" object received from server in the Google API V3 .NET Client Exception object
3 participants