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

How do you compare ErrorCodes? #83

Closed
wesleybowman opened this issue Apr 17, 2019 · 9 comments
Closed

How do you compare ErrorCodes? #83

wesleybowman opened this issue Apr 17, 2019 · 9 comments
Assignees

Comments

@wesleybowman
Copy link

If I have the following:

        results = ga_service.search(self.client_customer_id, query=query, page_size=page_size)

        try:
            yield from results

        except GoogleAdsException as ex:
            <'error handling here'>

There is a lot of information in ex. The examples in this repo print the exception request_id and the name of the status (ex.error.code().name)

Instead of accessing the status, which can be general, I want to access the actual failures,
ex.failure.errors.

for error in ex.failure.errors:
    if error.code == <'not sure what to do here'>:
        <'process individual failure'>

A use-case I have is that I want to trigger a certain code path if we receive a CLIENT_CUSTOMER_ID_INVALID and a different one if we get a CUSTOMER_NOT_ACTIVE. Using the StatusCode, both of these errors have a status of UNAUTHENTICATED.

My thoughts are that you need to create your own ErrorCode, but I am not having success with that using

error_code = client.get_type('ErrorCode', version='v1')

Any ideas?

@wesleybowman
Copy link
Author

I expected to be able to at least used the AuthenticationErrorEnum, but I cannot figure out how that might work either.

An example:

In [432]: exception.failure.errors[0].error_code                                                        
Out[432]: authentication_error: CUSTOMER_NOT_FOUND

In [433]: authentication_error = client.get_type('AuthenticationErrorEnum', version='v1')               

In [434]: authentication_error.CUSTOMER_NOT_FOUND                                                       
Out[434]: 8

In [435]: exception.failure.errors[0].error_code == authentication_error.CUSTOMER_NOT_FOUND             
Out[435]: False

@BenRKarl BenRKarl self-assigned this Apr 22, 2019
@BenRKarl
Copy link
Contributor

BenRKarl commented Apr 22, 2019

Hi @wesleybowman It looks like you're on the right track for finding the values you're trying to compare. UNAUTHENTICATED appears like it may be an auth error coming from the OAuth endpoint and not the Ads API. Are you able to authenticate properly and make successful search queries?

@wesleybowman
Copy link
Author

wesleybowman commented Apr 23, 2019

appears like it may be an auth error coming from the OAuth endpoint and not the Ads API. Are you able to authenticate properly and make successful search queries?

@BenRKarl I actually made the query crash so that I could try and compare different errors.
This isn't about the error that I am getting, it is more about the following:

We have customers that will get these different errors, and I want to send them down different code paths depending on which error they get. I think using a StatusCode doesn't give us enough fine grain control over these errors, so I was wondering the proper way to compare the errors.

@j-walker23
Copy link

j-walker23 commented May 1, 2019

@wesleybowman This is horrendous, but here is how i'm doing it.

Working in a python lib that has no built in interfaces or structure and instead by hidden proto files, is extremely difficult. It's been a long time since i couldn't get a single thing off of intellisense.
Pretty miserable.

But here is my crappy way.

# I don't show my helper methods, but their names are clear

# e is GoogleAdsException
code = e.failure.errors[0].error_code

# Figure out enum type and value by just using str(code) after giving up on a good way
# code eg. == request_error: INVALID_CUSTOMER_ID\n
# request_error in this case is enum type, then after colon is enum value
type_str, val = strip_all_spaces(str(code).strip()).split(':')
name = pascal_case(type_str)
type_ = client.get_type(f'{name}Enum', version='v1')
enum = getattr(type_, name)
enum_val = enum.Value(val)

# Then later in code i can do
from google.ads.google_ads.v1.services.enums import RequestErrorEnum
if RequestErrorEnum.RequestError.INVALID_CUSTOMER_ID == enum_val:
  # Now i can do something based on exact error enum 

@wesleybowman
Copy link
Author

@j-walker23

It's been a long time since i couldn't get a single thing off of intellisense.
Pretty miserable.

Agreed. It's been a difficult ride. What we ended up going with for now is the following:

        client = get_google_ads_client()
        authentication_error_enum = client.get_type('AuthenticationErrorEnum', version='v1')
        authorization_error_enum = client.get_type('AuthorizationErrorEnum', version='v1')
        request_error_enum = client.get_type('RequestErrorEnum', version='v1')

        for error in exception.failure.errors:

            error_code = error.error_code

            if error_code.authentication_error == authentication_error_enum.CUSTOMER_NOT_FOUND:
                raise AdwordsPermissionError('Customer Not Found', exception)

            elif error_code.authorization_error == authorization_error_enum.USER_PERMISSION_DENIED:
                raise AdwordsPermissionError('User Permission Denied', exception)

            elif error_code.request_error == request_error_enum.INVALID_CUSTOMER_ID:
                raise AdwordsInvalidClientCustomerId('Invalid Client Customer ID', exception)

Overall, fairly simple, but I am hoping there will be a better way in the future

@j-walker23
Copy link

j-walker23 commented May 2, 2019 via email

@BenRKarl
Copy link
Contributor

BenRKarl commented May 2, 2019

@j-walker23 @wesleybowman Thanks for coordinating with each other, and sorry for my delayed response. Is it accurate to describe the issue you're having generally as wanting to lookup the string name of an Enum field using the integer value pulled from a request object?

It looks like that's the core problem - if not, or if I'm leaving off other problems, please add on here.

The issue of doing this type of look up is definitely a major pain point. It's possible to do this a bit more easily by using the protobuf helpers, like this:

from google.protouf.json_format import MessageToDict

# e is GoogleAdsException
failure = e.failure
dict = MessageToDict(failure)

for error in dict['errors']:
    error_code = error['errorCode']

    if error_code['authenticationError'] == 'CUSTOMER_NOT_FOUND':
        raise AdwordsPermissionError('Customer Not Found', exception)

    if error_code['authorizationError'] == 'USER_PERMISSION_DENIED':
        raise AdwordsPermissionError('User Permission Denied', exception)

    if error_code['requestError'] == 'INVALID_CUSTOMER_ID':
        raise AdwordsInvalidClientCustomerId('Invalid Client Customer ID', exception)

Admittedly it's not a whole lot better, but at least you don't need to deal with the Enum protos directly.

One caveat about this solution is that it will generate errors if you are retrieving resource fields that contain a digit following an underscore, for example: video_quartile_25_rate. See this separate issue for more context: #30

I'm hoping to get functionality added to this library that will more easily expose the Enum field names so external helpers like MessageToDict aren't necessary. Hopefully that will come in the near future.

Let me know if you have any additional questions or thoughts here.

@wesleybowman
Copy link
Author

I'm hoping to get functionality added to this library that will more easily expose the Enum field names so external helpers like MessageToDict aren't necessary. Hopefully, that will come in the near future.

This would be wonderful!

and sorry for my delayed response.

No worries! We know you guys are busy. Keep up the good work 👍

Admittedly it's not a whole lot better, but at least you don't need to deal with the Enum protos directly.

I would like to extend this to also include an easier way to convert the StatusEnum into it's name instead of an int. For example, currently, to get the status as a string, we do the following:

campaign_status_enum = client.get_type('CampaignStatusEnum', version='v1').CampaignStatus.Name
for row in results:
    status: str = campaign_status_enum.CampaignStatus.Name(row.campaign.status)

IMHO, it would be nice if row.campaign.status had a value and name that could be accessed.

@BenRKarl
Copy link
Contributor

@wesleybowman Thank you for the explanation! I'm working on determining if there's a way that we can improve the interface for this. To be honest it won't be an easy change, so it might take some time. I'm going to close this issue since it's not directly related, but I'll open a new one to track progress against making Enums easier to work with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants