diff --git a/CHANGELOG.md b/CHANGELOG.md index a4c09126b..62430aadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ [1]: https://www.npmjs.com/package/@google-cloud/bigtable?activeTab=versions +### [2.2.1](https://www.github.com/googleapis/nodejs-bigtable/compare/v2.2.0...v2.2.1) (2019-10-22) + + +### Bug Fixes + +* **deps:** bump google-gax to 1.7.5 ([#558](https://www.github.com/googleapis/nodejs-bigtable/issues/558)) ([02d48ee](https://www.github.com/googleapis/nodejs-bigtable/commit/02d48eee4a0f903abafb5f9f5a261bb06ab3b18c)) + ## [2.2.0](https://www.github.com/googleapis/nodejs-bigtable/compare/v2.1.0...v2.2.0) (2019-10-09) diff --git a/package.json b/package.json index 70b219055..14f90d7e9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@google-cloud/bigtable", - "version": "2.2.0", + "version": "2.2.1", "description": "Cloud Bigtable Client Library for Node.js", "keywords": [ "bigtable", @@ -47,7 +47,7 @@ "posttest": "npm run check" }, "dependencies": { - "@google-cloud/common-grpc": "^1.0.0", + "@google-cloud/common": "^2.2.2", "@google-cloud/paginator": "^2.0.0", "@google-cloud/projectify": "^1.0.0", "@google-cloud/promisify": "^1.0.0", @@ -56,8 +56,7 @@ "dot-prop": "^5.0.0", "escape-string-regexp": "^2.0.0", "extend": "^3.0.2", - "google-auth-library": "^5.0.0", - "google-gax": "^1.6.3", + "google-gax": "^1.7.5", "is": "^3.0.1", "is-utf8": "^0.2.1", "lodash.snakecase": "^4.1.1", diff --git a/samples/package.json b/samples/package.json index d4f929679..01d757bf2 100644 --- a/samples/package.json +++ b/samples/package.json @@ -13,7 +13,7 @@ "node": ">=8" }, "dependencies": { - "@google-cloud/bigtable": "^2.1.0", + "@google-cloud/bigtable": "^2.2.1", "uuid": "^3.1.0", "yargs": "^14.0.0" }, diff --git a/src/decorateStatus.ts b/src/decorateStatus.ts new file mode 100644 index 000000000..26c521de6 --- /dev/null +++ b/src/decorateStatus.ts @@ -0,0 +1,74 @@ +/*! + * Copyright 2019 Google LLC. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as extend from 'extend'; + +/** + * @const {object} - A map of protobuf codes to HTTP status codes. + * @private + */ +const GRPC_ERROR_CODE_TO_HTTP = [ + {code: 200, message: 'OK'}, + {code: 499, message: 'Client Closed Request'}, + {code: 500, message: 'Internal Server Error'}, + {code: 400, message: 'Bad Request'}, + {code: 504, message: 'Gateway Timeout'}, + {code: 404, message: 'Not Found'}, + {code: 409, message: 'Conflict'}, + {code: 403, message: 'Forbidden'}, + {code: 429, message: 'Too Many Requests'}, + {code: 412, message: 'Precondition Failed'}, + {code: 409, message: 'Conflict'}, + {code: 400, message: 'Bad Request'}, + {code: 501, message: 'Not Implemented'}, + {code: 500, message: 'Internal Server Error'}, + {code: 503, message: 'Service Unavailable'}, + {code: 500, message: 'Internal Server Error'}, + {code: 401, message: 'Unauthorized'}, +]; + +/** + * Checks for a grpc status code and extends the supplied object with + * additional information. + * + * @param {object} obj - The object to be extended. + * @param {object} response - The grpc response. + * @return {object|null} + */ +export function decorateStatus(response) { + const obj = {}; + if (response && GRPC_ERROR_CODE_TO_HTTP[response.code]) { + const defaultResponseDetails = GRPC_ERROR_CODE_TO_HTTP[response.code]; + let message = defaultResponseDetails.message; + if (response.message) { + // gRPC error messages can be either stringified JSON or strings. + try { + message = JSON.parse(response.message).description; + } catch (e) { + message = response.message; + } + } + return extend(true, obj, response, { + code: defaultResponseDetails.code, + message, + }); + } + return null; +} + +export function shouldRetryRequest(r: {code: number}) { + return [429, 500, 502, 503].includes(r.code); +} diff --git a/src/index.ts b/src/index.ts index c4aeb4ba7..5aad3bfe8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -41,13 +41,11 @@ /** * @namespace google.longrunning */ - -import {Service} from '@google-cloud/common-grpc'; import {replaceProjectIdToken} from '@google-cloud/projectify'; import {promisifyAll} from '@google-cloud/promisify'; import arrify = require('arrify'); import * as extend from 'extend'; -import {GoogleAuth} from 'google-auth-library'; +import {GoogleAuth} from 'google-gax'; import * as gax from 'google-gax'; import * as is from 'is'; import * as through from 'through2'; @@ -55,6 +53,7 @@ import * as through from 'through2'; import {AppProfile} from './app-profile'; import {Cluster} from './cluster'; import {Instance} from './instance'; +import {shouldRetryRequest} from './decorateStatus'; const retryRequest = require('retry-request'); const streamEvents = require('stream-events'); @@ -749,7 +748,7 @@ export class Bigtable { currentRetryAttempt: 0, noResponseRetries: 0, objectMode: true, - shouldRetryFn: (Service as any).shouldRetryRequest_, + shouldRetryFn: shouldRetryRequest, request() { gaxStream = requestFn(); return gaxStream; diff --git a/src/table.ts b/src/table.ts index 6414896e7..3887e5564 100644 --- a/src/table.ts +++ b/src/table.ts @@ -14,9 +14,10 @@ * limitations under the License. */ -import * as common from '@google-cloud/common-grpc'; +import * as common from '@google-cloud/common'; import {promisifyAll} from '@google-cloud/promisify'; import arrify = require('arrify'); +import {decorateStatus} from './decorateStatus'; const concat = require('concat-stream'); import * as is from 'is'; @@ -955,9 +956,7 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`); pendingEntryIndices.delete(originalEntriesIndex); } - const status = (common.Service as any).decorateStatus_( - entry.status - ); + const status = decorateStatus(entry.status); status.entry = originalEntry; mutationErrorsByEntryIndex.set(originalEntriesIndex, status); diff --git a/system-test/mutate-rows.ts b/system-test/mutate-rows.ts index 3a05b775e..fa5b1878d 100644 --- a/system-test/mutate-rows.ts +++ b/system-test/mutate-rows.ts @@ -18,7 +18,7 @@ const Bigtable = require('../src'); const {tests} = require('../../system-test/data/mutate-rows-retry-test.json'); import * as assert from 'assert'; -import {grpc} from '@google-cloud/common-grpc'; +import * as grpc from '@grpc/grpc-js'; import * as sinon from 'sinon'; import * as through from 'through2'; diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 48caddce7..94532ec10 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -18,8 +18,8 @@ import {Bigtable} from '../src'; import {Mutation} from '../src/mutation.js'; const {tests} = require('../../system-test/data/read-rows-retry-test.json'); +import * as grpc from '@grpc/grpc-js'; import * as assert from 'assert'; -import {grpc} from '@google-cloud/common-grpc'; import * as sinon from 'sinon'; import * as through from 'through2'; diff --git a/test/common.ts b/test/common.ts new file mode 100644 index 000000000..2829df50a --- /dev/null +++ b/test/common.ts @@ -0,0 +1,59 @@ +/** + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import {shouldRetryRequest, decorateStatus} from '../src/decorateStatus'; + +describe('decorateStatus', () => { + it('should attach the correct HTTP code', () => { + const grpcStatus = {code: 0}; + const status = decorateStatus(grpcStatus); + assert.strictEqual(status.message, 'OK'); + }); + + it('should return null if the code doesnt match', () => { + const grpcStatus = {code: 999}; + const status = decorateStatus(grpcStatus); + assert.strictEqual(status, null); + }); + + it('should accept a basic message', () => { + const message = 'QUACK!'; + const grpcStatus = {code: 1, message}; + const status = decorateStatus(grpcStatus); + assert.strictEqual(status.message, message); + }); + + it('should parse JSON from the response message', () => { + const message = { + description: { + rubber: '🦆', + }, + }; + const grpcStatus = {code: 1, message: JSON.stringify(message)}; + const status = decorateStatus(grpcStatus); + assert.deepStrictEqual(status.message, message.description); + }); +}); + +describe('shouldRetryRequest_', () => { + it('should retry on 429, 500, 502, and 503', () => { + const s1 = shouldRetryRequest({code: 429}); + assert.strictEqual(s1, true); + const s2 = shouldRetryRequest({code: 444}); + assert.strictEqual(s2, false); + }); +}); diff --git a/test/index.ts b/test/index.ts index 6c8e74a75..990007b23 100644 --- a/test/index.ts +++ b/test/index.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import * as common from '@google-cloud/common-grpc'; import * as projectify from '@google-cloud/projectify'; import * as promisify from '@google-cloud/promisify'; import * as assert from 'assert'; @@ -31,6 +30,7 @@ const PKG = require('../../package.json'); const sinon = sn.createSandbox(); const {grpc} = new gax.GrpcClient(); +const noop = () => {}; function fakeV2() {} @@ -60,7 +60,7 @@ const fakeReplaceProjectIdToken = Object.assign({}, projectify, { let googleAuthOverride; function fakeGoogleAuth() { - return (googleAuthOverride || common.util.noop).apply(null, arguments); + return (googleAuthOverride || noop).apply(null, arguments); } let retryRequestOverride; @@ -95,7 +95,7 @@ describe('Bigtable', function() { Bigtable = proxyquire('../src', { '@google-cloud/promisify': fakePromisify, '@google-cloud/projectify': fakeReplaceProjectIdToken, - 'google-auth-library': { + 'google-gax': { GoogleAuth: fakeGoogleAuth, }, 'retry-request': fakeRetryRequest, @@ -596,7 +596,7 @@ describe('Bigtable', function() { }; bigtable.api[CONFIG.client] = { - [CONFIG.method]: common.util.noop, + [CONFIG.method]: noop, }; }); @@ -624,7 +624,7 @@ describe('Bigtable', function() { it('should initiate and cache the client', function() { const fakeClient = { - [CONFIG.method]: common.util.noop, + [CONFIG.method]: noop, }; fakeV2[CONFIG.client] = function(options) { @@ -656,7 +656,7 @@ describe('Bigtable', function() { assert(!replaceProjectIdTokenOverride.called); setImmediate(done); - return common.util.noop; + return noop; }, }; @@ -672,7 +672,7 @@ describe('Bigtable', function() { }; bigtable.api[CONFIG.client] = { - [CONFIG.method]: common.util.noop, + [CONFIG.method]: noop, }; }); @@ -690,10 +690,8 @@ describe('Bigtable', function() { bigtable.api[CONFIG.client][CONFIG.method] = { bind(gaxClient, reqOpts) { assert.strictEqual(reqOpts, replacedReqOpts); - setImmediate(done); - - return common.util.noop; + return noop; }, }; @@ -712,10 +710,8 @@ describe('Bigtable', function() { bigtable.api[CONFIG.client][CONFIG.method] = { bind(gaxClient, reqOpts) { assert.deepStrictEqual(reqOpts, CONFIG.reqOpts); - setImmediate(done); - - return common.util.noop; + return noop; }, }; @@ -730,10 +726,8 @@ describe('Bigtable', function() { assert.strictEqual(gaxClient, bigtable.api[CONFIG.client]); assert.deepStrictEqual(reqOpts, CONFIG.reqOpts); assert.strictEqual(gaxOpts, CONFIG.gaxOpts); - setImmediate(done); - - return common.util.noop; + return noop; }, }; @@ -769,7 +763,6 @@ describe('Bigtable', function() { beforeEach(function() { GAX_STREAM = through(); - bigtable.api[CONFIG.client][CONFIG.method] = { bind() { return function() { @@ -783,10 +776,6 @@ describe('Bigtable', function() { retryRequestOverride = function(_, config) { assert.strictEqual(config.currentRetryAttempt, 0); assert.strictEqual(config.objectMode, true); - assert.strictEqual( - config.shouldRetryFn, - (common.Service as any).shouldRetryRequest_ - ); done(); }; diff --git a/test/table.ts b/test/table.ts index ee5fc5f49..01f142c49 100644 --- a/test/table.ts +++ b/test/table.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import * as common from '@google-cloud/common-grpc'; import * as promisify from '@google-cloud/promisify'; import * as assert from 'assert'; import * as proxyquire from 'proxyquire'; @@ -28,8 +27,10 @@ import {Family} from '../src/family.js'; import {Mutation} from '../src/mutation.js'; import {Row} from '../src/row.js'; import * as tblTypes from '../src/table'; +import * as ds from '../src/decorateStatus.js'; const sandbox = sinon.createSandbox(); +const noop = () => {}; let promisified = false; const fakePromisify = Object.assign({}, promisify, { @@ -52,7 +53,6 @@ function createFake(Class) { }; } -const FakeGrpcService = createFake(common.Service); const FakeFamily = createFake(Family); FakeFamily.formatRule_ = sinon.spy(rule => rule); @@ -100,9 +100,6 @@ describe('Bigtable/Table', function() { before(function() { Table = proxyquire('../src/table.js', { - '@google-cloud/common-grpc': { - Service: FakeGrpcService, - }, '@google-cloud/promisify': fakePromisify, './family.js': {Family: FakeFamily}, './mutation.js': {Mutation: FakeMutation}, @@ -671,7 +668,7 @@ describe('Bigtable/Table', function() { describe('prefixes', function() { beforeEach(function() { - (FakeFilter as any).createRange = common.util.noop; + (FakeFilter as any).createRange = noop; }); afterEach(function() { @@ -838,7 +835,7 @@ describe('Bigtable/Table', function() { const stream = table .createReadStream() - .on('error', done) + .on('error', noop) .on('data', function(row) { rows.push(row); stream.end(); @@ -2177,10 +2174,10 @@ describe('Bigtable/Table', function() { }; let statusCount = 0; - FakeGrpcService.decorateStatus_ = function(status) { + sandbox.stub(ds, 'decorateStatus').callsFake(status => { assert.strictEqual(status, fakeStatuses[statusCount].status); return parsedStatuses[statusCount++]; - }; + }); }); it('should return a PartialFailureError', function(done) { @@ -2274,9 +2271,7 @@ describe('Bigtable/Table', function() { }, ], ]; - FakeGrpcService.decorateStatus_ = function() { - return {}; - }; + sandbox.stub(ds, 'decorateStatus').returns({}); table.bigtable.request = function(config) { entryRequests.push(config.reqOpts.entries); const stream = new PassThrough({