Skip to content

Commit

Permalink
gaarf-js@2.11.3 Fix: GoogleAdsRpcApiClient: removed gRPC error codes …
Browse files Browse the repository at this point in the history
…parsing, all errors not from the API are treated as transient with enabled retry

Change-Id: I853748bc53f1b28e3d317a2ce7d9671c1c554911
  • Loading branch information
evil-shrike committed Aug 15, 2024
1 parent 08aed30 commit bb3e2d0
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 26 deletions.
23 changes: 14 additions & 9 deletions js/dist/lib/ads-api-client.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/dist/lib/ads-api-client.js.map

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions js/dist/lib/logger-factory.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/dist/lib/logger-factory.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/dist/lib/logger.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions js/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "google-ads-api-report-fetcher",
"version": "2.11.2",
"version": "2.11.3",
"description": "Google Ads API Report Fetcher (gaarf)",
"main": "./dist/index.js",
"types": "./src/index.ts",
Expand Down
25 changes: 14 additions & 11 deletions js/src/lib/ads-api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,15 @@ export class GoogleAdsRpcApiClient
} else {
// it could be an error from gRPC
// we expect an Error instance with interface of ServiceError from @grpc/grpc-js library
// see status codes: https://grpc.github.io/grpc/core/md_doc_statuscodes.html
if (
(<any>error).code === 14 /* UNAVAILABLE */ ||
(<any>error).details === "The service is currently unavailable" ||
(<any>error).code === 8 /* RESOURCE_EXHAUSTED */ ||
(<any>error).code === 4 /* DEADLINE_EXCEEDED */ ||
(<any>error).code === 13 /* INTERNAL */
) {
(<any>error).retryable = true;
}
// We used to handle only a subset of error by error code
// (see status codes: https://grpc.github.io/grpc/core/md_doc_statuscodes.html)
// particularly 14 (unavailble), 13 (internal server error),
// 8(RESOURCE_EXHAUSTED), 4 (DEADLINE_EXCEEDED)
// But there was always something new that we didn't expect, so
// in the end it seems much safer to treat any error not from the API
// server as transient and allow retrying.
(<any>error).retryable = true;
return error;
}
}

Expand Down Expand Up @@ -292,13 +291,17 @@ export class GoogleAdsRpcApiClient
}

async *executeQueryStream(query: string, customerId: string) {
const customer = this.getCustomer(customerId);
try {
const customer = this.getCustomer(customerId);
// As we return an AsyncGenerator here we can't use executeWithRetry,
// instead usages of the method should be wrapped with executeWithRetry
// NOTE: we're iterating over the stream instead of returning it
// for the sake of error handling
const stream = customer.queryStream(query);
this.logger.debug(`Created AsyncGenerator from queryStream`, {
customerId,
query,
});
for await (const row of stream) {
yield row;
}
Expand Down
10 changes: 10 additions & 0 deletions js/src/lib/logger-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ export function createCloudLogger(): winston.Logger {
type: "cloud_function",
},
useMessageField: false,
// setting redirectToStdout:true actually disables using Logging API,
// and simply dumps entries to stdout where the logger agent
// parses them and redirect to Logging.
// It's the only way to overcome sparodic errors
// during calling Logging API:
// "GoogleError: Total timeout of API google.logging.v2.LoggingServiceV2
// exceeded 60000 milliseconds before any response was received"
// See https://github.com/googleapis/nodejs-logging/issues/1185
// And even recommended in
// https://cloud.google.com/nodejs/docs/reference/logging-winston/latest#alternative-way-to-ingest-logs-in-google-cloud-managed-environments
redirectToStdout: true,
handleRejections: LOG_LEVEL === "debug",
}),
Expand Down

0 comments on commit bb3e2d0

Please sign in to comment.