Opteo / google-ads-api

Google Ads API client library for Node.js
https://opteo.com
MIT License
270 stars 90 forks source link

reportStream does not automatically end when there is no further row #420

Closed danielglh closed 1 year ago

danielglh commented 2 years ago

We started to experience the issue this week. Sometimes the reportStream for loop didn't end and just hang there when there's really no further data to fetch.

Code is like this:

const stream = customer.reportStream({
  entity: "ad_group_criterion",
  attributes: [
    "ad_group_criterion.keyword.text", 
    "ad_group_criterion.status",
  ],
  constraints: {
    "ad_group_criterion.type": enums.CriterionType.KEYWORD,
  },
});

let count = 0;
for await (const row of stream) {
    // Do something with the row data
    count += 1
    console.log(count);
}

When I ran the code, it hung after the count reached the exact count of keywords in that account (I got the count from the Google Ads dashboard).

Right now we are trying the hack to use reportCount to get the count first and then break from the loop when we know all data has been fetched, but previously we don't need to do that, and it's not happening to all reportStream calls.

The hacking way:

const count = await customer.reportCount({
  entity: "ad_group_criterion",
  attributes: [
    "ad_group_criterion.keyword.text", 
    "ad_group_criterion.status",
  ],
  constraints: {
    "ad_group_criterion.type": enums.CriterionType.KEYWORD,
  },
});

const stream = customer.reportStream({
  entity: "ad_group_criterion",
  attributes: [
    "ad_group_criterion.keyword.text", 
    "ad_group_criterion.status",
  ],
  constraints: {
    "ad_group_criterion.type": enums.CriterionType.KEYWORD,
  },
});

let rowCount = 0;
for await (const row of stream) {
    // Do something with the row data
    rowCount += 1
    console.log(rowCount);
    if (rowCount >= count) {
        break;
    }
}

I'm not sure if this is a hack or it's the correct way to do so. If it's not, what would be the correct way to handle streams?

danielglh commented 2 years ago

BTW, I wrote a simple helper function for this fix

import _ from 'lodash';

export const reportStream = async function* (customer, queryOptions) {
  const totalCount = await customer.reportCount(_.cloneDeep(queryOptions));
  const stream = customer.reportStream(_.cloneDeep(queryOptions));
  let rowCount = 0;
  for await (const row of stream) {
    yield row;
    rowCount += 1;
    if (rowCount >= totalCount) {
      break;
    }
  }
}

It seems to do the trick but I'd still like to confirm whether this is the right way to solve it. Seem kinda hacky.