Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autopagination does not throw backend errors appropriately #1373

Open
sofisl opened this issue Oct 5, 2022 · 0 comments
Open

Autopagination does not throw backend errors appropriately #1373

sofisl opened this issue Oct 5, 2022 · 0 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@sofisl
Copy link
Contributor

sofisl commented Oct 5, 2022

Investigating: googleapis/nodejs-datacatalog#296

It seems like, when autoPagination is on, and you throw an error, it does not stop getting resources. To reproduce:

  1. call searchAssets.js in datacatalog (datacatalog has a lot of resources, we're trying to get a large response):
'use strict';

async function main(projectId) {
  // [START data_catalog_search_assets]
  // Import the Google Cloud client library.
  const {DataCatalogClient} = require('@google-cloud/datacatalog').v1;
  const datacatalog = new DataCatalogClient();

  async function searchAssets(projectId) {
    // Search data assets.

    /**
     * TODO(developer): Uncomment the following lines before running the sample.
     */
    // const projectId = 'my_project'; // Google Cloud Platform project

    // Create request.
    const scope = {
      includeProjectIds: [projectId],
      // Alternatively, search using Google Cloud Organization scopes.
      //includeOrgIds: ["google.com"],
    };

    const request = {
      scope: scope,
      pageSize: 50,
      includeGcpPublicDatasets: true,
      orderBy: 'relevance',
    };

    const [result] = await datacatalog.searchCatalog(request);

    console.log(`Found ${result.length}`);
    console.log('Datasets:');
    result.forEach(dataset => {
      console.log(dataset.relativeResourceName);
    });
  }
  searchAssets();
  // [END data_catalog_search_assets]
}
main(...process.argv.slice(2));

Then, in gax (in pagedApiCaller.ts,

callback(err, resources, nextPageRequest, response);
):

Add a global counter, and these statements:

            counter++;
            callback(err, resources, nextPageRequest, response);
            if (counter === 5) { const err = new googleError_1.GoogleError('error'); err.code = 8; /* resource exhausted */ callback(err); return; }

It will continue grabbing pages indefinitely.

In addition to fixing this bug, we should add unit tests here.

@sofisl sofisl added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Oct 5, 2022
@sofisl sofisl self-assigned this Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

1 participant