V25.1 - #549
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7feda2ec9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const client = new protoService({ | ||
| sslCreds: this.getCredentials(), | ||
| universeDomain: "googleapis.com", | ||
| ...this.clientOptions.grpc_channel_options, |
There was a problem hiding this comment.
Include channel options in the service cache key
When two GoogleAdsApi instances use the same client ID and refresh token but different grpc_channel_options, the module-level cache key at line 149 treats their service clients as identical. After the first instance populates the cache, the second returns that existing client before this spread is evaluated, so its keepalive, proxy, message-size, or other channel settings are silently ignored for up to ten minutes. Partition the cache by the channel configuration or scope cached services to the owning client instance.
Useful? React with 👍 / 👎.
Summary
Upgrades the library to Google Ads API v25.1, depending on
google-ads-node@25.1.0-beta.1,google-gax@^6.3.0andgoogle-auth-library@^11. Published under thebetadist-tag as25.1.0-beta.0, then25.1.0-beta.1with the review fixes, for testing ahead of the stable release.Alongside the version upgrade, the package now ships CommonJS and ES module builds behind an export map, and two open issues are addressed.
Version upgrade
engines, CI matrix 22.x and 24.x)MultiPartyAuthReviewService; new resource names for the lift measurement resources andMultiPartyAuthReview; 21 new enumsgoogleapis.comuniverse domain so the new auth stack does not walk the application-default-credentials chain (about three seconds per new service client, plus aMetadataLookupWarningoutside Google Cloud)Headersreturn value to the plain object@grpc/grpc-jsexpectsESM export map
build/cjsandbuild/esmare emitted from the same source;exportsmaps.,./enums,./fieldsand./package.jsonwithimport/requireconditions and their own typesgoogleAdsVersionis exported from the package root.jsextensions and the codegen template emits them too;stream-chain/stream-jsonare imported as defaults because Node's ESM loader cannot see their named exportstests/package.mjs: ESM import, CommonJS require, subpaths, deep imports rejected) and@arethetypeswrong/cli@0.18.5 --profile node16 --pack .postcompile), which keeps their diffs reviewableBreaking changes
google-ads-api/build/src/protos/autogen/enums) no longer resolve; use the root,google-ads-api/enumsorgoogle-ads-api/fieldsCampaignLifecycleGoalService,CustomerLifecycleGoalService, the two lifecycle goal resources, and theCustomerAcquisitionOptimizationModeenumFixes
getGoogleAdsError()returns the original error instead of throwing aTypeErrorwhen noGoogleAdsFailuretrailer is presentgrpc_channel_optionsclient option forwards gRPC channel settings (keepalive etc.) to every service client; cached service clients are keyed by client id, refresh token and these options, andsslCreds/universeDomainstay under library controljws; the repo lockfile now resolves 4.0.1 and the changelog tells consumers to refresh theirsprotobufjs7.6.6,@grpc/grpc-js1.14.4,axios1.20.0,form-data4.0.6,follow-redirects1.16,@protobufjs/utf81.1.1), which closes every runtime advisoryyarn auditreported exceptstream-json(fix requires its 3.x major; the affected filter classes are not used here).ts-jestis pinned at 29.3.4 because 29.4 resolves axios's dual typings inconsistently and breaks a specTest plan
yarn lint,yarn test(197 tests) on Node 22.23 and 24.19yarn build; arethetypeswrong and publint clean on the packed tarball; ESM and CommonJS runtime smoke through the export map25.1.0-beta.0under thebetadist-tag25.1.0-beta.1(depends on google-ads-node 25.1.0-beta.1; carries the review fixes) under thebetadist-tag and test downstream, including OAuth token refresh on Node 24.17+