CredentialsWrapper doesn't use authHttpClient when updating meta data

[stefanzweifel]

Hi there!

I'm in a Laravel PHP project where I would like to replace the default HttpClient used by the google-ads SDK and gax-php to be the Laravel Http Client.

I noticed that CredentialsWrapper doesn't pass $this->authHttpHandler to $this->credentialsFetcher->updateMetaData() here.

I didn't understand how CredentialsWrapper is generated (through protobuf or manually), so I didn't send in a pull request.

Passing $this->authHttpHandler to the updateMetadata method did the trick for me.

 // Call updateMetadata to take advantage of self-signed JWTs
 if ($this->credentialsFetcher instanceof UpdateMetadataInterface) {
-    return $this->credentialsFetcher->updateMetadata([], $audience);
+    return $this->credentialsFetcher->updateMetadata([], $audience, $this->authHttpHandler);
 }

Environment details

• OS: macOS
• PHP version: 8.2
• Package name and version: v1.30.0

Steps to reproduce

1. Create a custom HttpHandler class/closure
2. Add the custom HttpHandler to the client options
3. When making a request to a Google Ads API endpoint, the request to fetch the OAuth Token is done using the default Guzzle7HttpHandler instead of the custom HttpHandler.

Code example

Here's a short code example using both the Google Ads SDK and gax-php. The print_r statement in the custom HttpHandler will only output the request URL to https://googleads.googleapis.com/* but not to https://oauth2.googleapis.com/token.

<?php

use Google\Ads\GoogleAds\Lib\OAuth2TokenBuilder;
use Google\Ads\GoogleAds\Lib\V16\GoogleAdsClientBuilder;
use Google\Ads\GoogleAds\V16\Services\Client\GoogleAdsServiceClient;
use Google\Ads\GoogleAds\V16\Services\SearchGoogleAdsRequest;
use GuzzleHttp\Promise\PromiseInterface;
use Psr\Http\Message\RequestInterface;

$OAuth2Credential = (new OAuth2TokenBuilder())
    ->withClientId($clientId)
    ->withClientSecret($clientSecret)
    ->withRefreshToken($googleRefreshToken)
    ->build();

$googleAdsClient = (new GoogleAdsClientBuilder())
    ->withOAuth2Credential($OAuth2Credential)
    ->withDeveloperToken($developerToken)
    ->withLoginCustomerId($customerId)
    ->withTransport('rest')
    ->build();

$laravelHttpHandler = new class () {
    public function __invoke(RequestInterface $request, array $options = []): ?PromiseInterface
    {
        /** Implement Http Request using Laravels Http Client */
        print_r($request->getUri()->__toString());
    }
};

$clientOptions = $googleAdsClient->getGoogleAdsClientOptions();

// Update Client Options and override httpHandler and authHttpHandler.
$clientOptions['transportConfig']['rest'] = [
    'httpHandler' => $laravelHttpHandler,
];
$clientOptions['credentialsConfig']['authHttpHandler'] = $laravelHttpHandler;

// Pass updated client Options to Google Ads Service Client.
$googleAdsServiceClient = new GoogleAdsServiceClient($clientOptions);

$query = "SELECT campaign.id, campaign.name, segments.date FROM campaign WHERE campaign.status IN ('ENABLED', 'PAUSED', 'REMOVED') AND segments.date during TODAY";
$searchRequest = new SearchGoogleAdsRequest([
    'customer_id' => $customerId,
    'query' => $query,
]);

$response = $googleAdsServiceClient->search($searchRequest);

// Handle Response here …

---

This is my first time interacting with the suite of Google SDKs. Not sure if there is a better way to accomplish this. (I couldn't find documentation on how to replace the HttpClient and digging through the code I coudln't really make out the difference between "httpHandler" and "transport")

[yash30201]

Hi @stefanzweifel, thanks for bringing this to our attention and for the excellent debugging work! Your proposed fix looks spot-on.

I didn't understand how CredentialsWrapper is generated (through protobuf or manually), so I didn't send in a pull request.

You're right to wonder about how CredentialsWrapper is generated. It's created dynamically within the gax-php library (see here).

I noticed that CredentialsWrapper doesn't pass $this->authHttpHandler to $this->credentialsFetcher->updateMetaData() here.

It does appear that the authHttpHandler argument is missing in the updateMetadata call, reason being the other calls to auth (fetchAuthToken) have the authHttpHandler argument.

I would encourage you to submit a pull request with your fix! It's a valuable contribution.

[norberttech]

hey @yash30201, since you already confirmed that the code is broken, and since @stefanzweifel moved to another solution before his code could get merged, maybe you could push that fix?

[norberttech]

@yash30201 or anyone else maintaining this repository, would it be possible that one of you who already went through this CLA process to apply suggested by @stefanzweifel patch and fix bug described in this PR?

Also, @yash30201 to clarify things, this is a bug, not a feature request. Not being able to replace default implementation of HttpClient prevents anyone who is using this library to for example mock responses - this is a severe bug, not a nice to have feature.

[norberttech]

is this repo even maintained?

[yash30201]

Hi @norberttech ,

Sorry for the long delay in the reply. Folks have been out of office so it took time coming to this one.

Also, @yash30201 to clarify things, this is a bug, not a feature request.

Makes sense, I've updated the tag