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

Concurrency and sdk mutation #64

Closed
distrill opened this issue Jul 5, 2018 · 4 comments
Closed

Concurrency and sdk mutation #64

distrill opened this issue Jul 5, 2018 · 4 comments
Labels

Comments

@distrill
Copy link

distrill commented Jul 5, 2018

We have a bunch of jobs that we would like to run concurrently. However, if we do so we have no guarantee that the appropriate token will be used. Simple reproduction case follows:

const adsSdk = require('facebook-nodejs-business-sdk');
const Promise = require('bluebird');

const { AdAccount } = adsSdk;

async function runForAccount(actId, token, waitTime) {
  adsSdk.FacebookAdsApi.init(token);

  console.log('waiting for', waitTime, 'ms (', token, ')');
  await Promise.delay(waitTime);
  const account = new AdAccount(actId);

  try {
    const info = await account.read([AdAccount.Fields.name, AdAccount.Fields.age]);
    console.log(info._data);
  } catch (err) {
    console.log('bad token failed');
  }
}

const actId = '<act_id>';
const badToken = '<bad_token>';
const goodToken = '<good_token>';

runForAccount(actId, goodToken, 2000);
runForAccount(actId, badToken, 10);

desired output:

bad token failed
{ id: '<id>', name: '<name>', age: '<age>' }

actual output

bad token failed
bad token failed

Is there some way to work around this so that we can have multiple instances of the sdk running at the same time? Or is there a plan to support this kind of use in the future? Javascript in particular is heavily asynchronous, this side-effect behavior is not desirable. We've resorted to making the requests directly ourselves for now but would prefer to use the official sdk if possible.

@JoakimLandberg
Copy link

JoakimLandberg commented Aug 7, 2018

Looks like you'll have to explicitly create an instance of FacebookAdsApi and use that as a constructor argument, rather than relying on the default instance created by the static init method.

const api = new FacebookAdsApi(token);

const account = new AdAccount(actId, {}, null, api);

@imayolas
Copy link

I would like to suggest, for the good of the sdk and the developers using it, to better document this method for instantiating the API.

I'm sure that a common use case of this sdk is for developers who create apps who have multiple clients. Therefore, for each client needs a different user_token. In the README, only mentions the instantiation method to set up a global user_token for all the API calls.

It's easy to omit this detail and detail jump into other areas of code. In our case, the code was similar to the example provided & it never failed until we run it concurrently with different consumers who required different user tokens. During development, you don't have concurrency and this error can be easily leaked. Idem with tests.

Thanks.

@stale
Copy link

stale bot commented Jan 14, 2020

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Jan 14, 2020
@stale
Copy link

stale bot commented Jan 21, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this as completed Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants