feat: add helpful warnings on paged calls (#1668)

* feat: warn on paged calls for conflicting settings

* add other warnings and fix lint

gax/src/paginationCalls/pageDescriptor.ts

@@ -28,6 +28,7 @@ import {
 import {Descriptor} from '../descriptor';
 import {CallSettings} from '../gax';
 import {NormalApiCaller} from '../normalCalls/normalApiCaller';
+import {warn} from '.././warnings';
 
 import {PagedApiCaller} from './pagedApiCaller';
 
@@ -63,6 +64,13 @@ export class PageDescriptor implements Descriptor {
     request: {},
     options: CallSettings
   ): Transform {
+    if (options?.autoPaginate) {
+      warn(
+        'autoPaginate true',
+        'Autopaginate will always be set to false in stream paging methods. See more info at https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure paging calls',
+        'AutopaginateTrueWarning'
+      );
+    }
     const stream = new PassThrough({objectMode: true});
     options = Object.assign({}, options, {autoPaginate: false});
     const maxResults = 'maxResults' in options ? options.maxResults : -1;
@@ -137,6 +145,13 @@ export class PageDescriptor implements Descriptor {
     request: RequestType,
     options?: CallSettings
   ): AsyncIterable<{} | undefined> {
+    if (options?.autoPaginate) {
+      warn(
+        'autoPaginate true',
+        'Autopaginate will always be set to false in Async paging methods. See more info at https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure paging calls',
+        'AutopaginateTrueWarning'
+      );
+    }
     options = Object.assign({}, options, {autoPaginate: false});
     const iterable = this.createIterator(apiCall, request, options);
     return iterable;

gax/src/paginationCalls/pagedApiCaller.ts

@@ -28,6 +28,7 @@ import {CallOptions} from '../gax';
 import {GoogleError} from '../googleError';
 import {PageDescriptor} from './pageDescriptor';
 import {ResourceCollector} from './resourceCollector';
+import {warn} from '.././warnings';
 
 export class PagedApiCaller implements APICaller {
   pageDescriptor: PageDescriptor;
@@ -152,6 +153,15 @@ export class PagedApiCaller implements APICaller {
       ongoingCall.call(apiCall, request);
       return;
     }
+    console.log('155', request, settings);
+    if (request.pageSize && settings.autoPaginate) {
+      warn(
+        'autoPaginate true',
+        'Providing a pageSize without setting autoPaginate to false will still return all results. See https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure manual paging',
+        'AutopaginateTrueWarning'
+      );
+    }
+    console.log(request.pageSize && settings.autoPaginate);
 
     const maxResults = settings.maxResults || -1;
 

gax/test/test-application/src/index.ts

@@ -93,6 +93,7 @@ async function testShowcase() {
   await testExpand(grpcClient);
   await testPagedExpand(grpcClient);
   await testPagedExpandAsync(grpcClient);
+  await testPagedExpandAutopaginateOff(grpcClient);
   await testCollect(grpcClient);
   await testChat(grpcClient);
   await testWait(grpcClient);
@@ -101,6 +102,7 @@ async function testShowcase() {
   await testExpand(restClient); // REGAPIC supports server streaming
   await testPagedExpand(restClient);
   await testPagedExpandAsync(restClient);
+  await testPagedExpandAutopaginateOff(restClient);
   await testCollectThrows(restClient); // REGAPIC does not support client streaming
   await testChatThrows(restClient); // REGAPIC does not support bidi streaming
   await testWait(restClient);
@@ -109,6 +111,7 @@ async function testShowcase() {
   await testExpand(restClientCompat); // REGAPIC supports server streaming
   await testPagedExpand(restClientCompat);
   await testPagedExpandAsync(restClientCompat);
+  await testPagedExpandAutopaginateOff(restClientCompat);
   await testCollectThrows(restClientCompat); // REGAPIC does not support client streaming
   await testChatThrows(restClientCompat); // REGAPIC does not support bidi streaming
   await testWait(restClientCompat);
@@ -167,6 +170,7 @@ async function testShowcase() {
   await testExpand(grpcClientWithServerStreamingRetries);
   await testPagedExpand(grpcClientWithServerStreamingRetries);
   await testPagedExpandAsync(grpcClientWithServerStreamingRetries);
+  await testPagedExpandAutopaginateOff(grpcClientWithServerStreamingRetries);
   await testCollect(grpcClientWithServerStreamingRetries);
   await testChat(grpcClientWithServerStreamingRetries);
   await testWait(grpcClientWithServerStreamingRetries);
@@ -526,6 +530,31 @@ async function testPagedExpandAsync(client: EchoClient) {
   assert.deepStrictEqual(words, response);
 }
 
+async function testPagedExpandAutopaginateOff(client: EchoClient) {
+  const words = ['nobody', 'ever', 'reads', 'test', 'input'];
+  const request = {
+    content: words.join(' '),
+    pageSize: 2,
+  };
+  const timer = setTimeout(() => {
+    throw new Error('End-to-end testPagedExpand method fails with timeout');
+  }, 12000);
+  const [resultArray, nextPageRequest] = await client.pagedExpand(request, {
+    autoPaginate: false,
+  });
+  clearTimeout(timer);
+  const result = resultArray.map(r => r.content);
+  assert.deepStrictEqual(words.slice(0, 2), result);
+  // manually paginate
+  const [response2] = await client.pagedExpand(nextPageRequest!, {
+    autoPaginate: false,
+  });
+
+  clearTimeout(timer);
+  const result2 = response2.map(r => r.content);
+  assert.deepStrictEqual(words.slice(2, 4), result2);
+}
+
 async function testCollect(client: EchoClient) {
   const words = ['nobody', 'ever', 'reads', 'test', 'input'];
   const promise = new Promise<string>((resolve, reject) => {

gax/test/unit/pagedIteration.ts

@@ -29,6 +29,7 @@ import {describe, it, beforeEach} from 'mocha';
 import * as util from './utils';
 import {Stream} from 'stream';
 import * as gax from '../../src/gax';
+import * as warnings from '../../src/warnings';
 
 describe('paged iteration', () => {
   const pageSize = 3;
@@ -58,6 +59,31 @@ describe('paged iteration', () => {
     }
   }
 
+  it('warns when pageSize is configured without configuring autoPaginate', done => {
+    const warnStub = sinon.stub(warnings, 'warn');
+
+    const apiCall = util.createApiCall(func, createOptions);
+    const expected: Array<{}> = [];
+    for (let i = 0; i < pageSize * pagesToStream; ++i) {
+      expected.push(i);
+    }
+    apiCall({pageSize: pageSize}, undefined)
+      .then(results => {
+        assert.ok(Array.isArray(results));
+        assert.deepStrictEqual(results[0], expected);
+        assert.strictEqual(warnStub.callCount, 1);
+        assert(
+          warnStub.calledWith(
+            'autoPaginate true',
+            'Providing a pageSize without setting autoPaginate to false will still return all results. See https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure manual paging',
+            'AutopaginateTrueWarning'
+          )
+        );
+        warnStub.restore();
+        done();
+      })
+      .catch(done);
+  });
   it('returns an Array of results', done => {
     const apiCall = util.createApiCall(func, createOptions);
     const expected: Array<{}> = [];
@@ -225,7 +251,43 @@ describe('paged iteration', () => {
       );
       assert.strictEqual(resources.length, 10);
     });
+    it('returns an iterable, count to 10, warns if autopaginate is specified', async () => {
+      const warnStub = sinon.stub(warnings, 'warn');
+
+      const spy = sinon.spy(func);
+      const apiCall = util.createApiCall(spy, createOptions);
+
+      async function iterableChecker(iterable: AsyncIterable<{} | undefined>) {
+        let counter = 0;
+        const resources = [];
+        for await (const resource of iterable) {
+          counter++;
+          resources.push(resource);
+          if (counter === 10) {
+            break;
+          }
+        }
+        return resources;
+      }
 
+      const settings = new gax.CallSettings(
+        (createOptions && createOptions.settings) || {}
+      );
+      settings.autoPaginate = true;
+      const resources = await iterableChecker(
+        descriptor.asyncIterate(apiCall, {}, settings)
+      );
+      assert.strictEqual(resources.length, 10);
+      assert.strictEqual(warnStub.callCount, 1);
+      assert(
+        warnStub.calledWith(
+          'autoPaginate true',
+          'Autopaginate will always be set to false in Async paging methods. See more info at https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure paging calls',
+          'AutopaginateTrueWarning'
+        )
+      );
+      warnStub.restore();
+    });
     it('does not stop on empty resources list', async () => {
       function func(
         request: {pageToken?: number},
@@ -337,8 +399,32 @@ describe('paged iteration', () => {
         0
       );
     });
+    it('ignores autoPaginate options and warns, but respects others', done => {
+      const warnStub = sinon.stub(warnings, 'warn');
 
-    it('ignores autoPaginate options, but respects others', done => {
+      // Specifies autoPaginate: true, which will be ignored, and maxResults:
+      // pageSize which will be used.
+      const options = {maxResults: pageSize, autoPaginate: true};
+      streamChecker(
+        // @ts-ignore incomplete options
+        descriptor.createStream(apiCall, {}, options),
+        () => {
+          assert.strictEqual(spy.callCount, 1);
+          assert.strictEqual(warnStub.callCount, 1);
+          assert(
+            warnStub.calledWith(
+              'autoPaginate true',
+              'Autopaginate will always be set to false in stream paging methods. See more info at https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md#auto-pagination for more information on how to configure paging calls',
+              'AutopaginateTrueWarning'
+            )
+          );
+          warnStub.restore();
+        },
+        done,
+        0
+      );
+    });
+    it('ignores autoPaginate options but respects others', done => {
       // Specifies autoPaginate: false, which will be ignored, and maxResults:
       // pageSize which will be used.
       const options = {maxResults: pageSize, autoPaginate: false};