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

Bug: pbjs.refreshUserIds() Fails to Execute Callback in identityLinkIdSystem, Blocking Submodule ID Refresh and Triggering Error #12243

Closed
yagovelazquezfreestar opened this issue Sep 16, 2024 · 5 comments

Comments

@yagovelazquezfreestar
Copy link
Contributor

yagovelazquezfreestar commented Sep 16, 2024

Type of issue

Bug

Description

Screenshot 2024-09-15 at 21 16 25

This bug happens when calling the method pbjs.refreshUserIds() and is related to the identityLinkIdSystem module.
When attempting to getEnvelope, if a setRetryCookie has already been set, the userId callback that signals the submodule completion is not executed. This disrupts the process, preventing all subsequent submodules from refreshing their IDs. It will also throw the error you see in the screenshot. That's the main issue.
Thats the function I'm talking about:

function getEnvelope(url, callback, configParams) {
  const callbacks = {
    success: response => {
      let responseObj;
      if (response) {
        try {
          responseObj = JSON.parse(response);
        } catch (error) {
          utils.logInfo(error);
        }
      }
      callback((responseObj && responseObj.envelope) ? responseObj.envelope : '');
    },
    error: error => {
      utils.logInfo(`identityLink: identityLink: ID fetch encountered an error`, error);
      callback();
    }
  };

  if (!configParams.notUse3P && !storage.getCookie('_lr_retry_request')) {
    setRetryCookie();
    utils.logInfo('identityLink: A 3P retrieval is attempted!');
    setEnvelopeSource(false);
    ajax(url, callbacks, undefined, { method: 'GET', withCredentials: true });
  }
} 

But, I also see that when trying to getEnvelopeFromStorage it tries to get from this cookie or local storage, but there isn't a part in this module that sets this cookie or local storage envelope. Am I missing something here?

let rawEnvelope = storage.getCookie(liverampEnvelopeName) || storage.getDataFromLocalStorage(liverampEnvelopeName);

This is my identityLinkConfig inside pbjs.getConfig().userSync.userIds

{
    "name": "identityLink",
    "params": {
        "pid": "106"
    },
    "storage": {
        "expires": "30",
        "name": "idl_env",
        "type": "cookie",
        "refreshInSeconds": "1800"
    }
}

Steps to reproduce

  1. Add in prebid configuration of userSync.userIds, the following config:
[{
    "name": "identityLink",
    "params": {
        "pid": "106"
    },
    "storage": {
        "expires": "30",
        "name": "idl_env",
        "type": "cookie",
        "refreshInSeconds": "1800"
    }
}]

  1. Do a pbjs.refreshUserIds()
  2. Wait for the _lr_retry_request cookies to be set as true
  3. Do a pbjs.refreshUserIds() again and check the console error
Screenshot 2024-09-15 at 21 16 25

Expected results

  1. The submodules user refresh ids keep executing without being disrupted.
  2. I'm not sure if it's not wanted to be made another ajax request. If that's the case and it was made to be only one ajax request and therefore this cookie is set to prevent more retry requests, it should have an else with a callback inside being executed there to keep the flow going.
  3. If it's okay to make more ajax requests, the cookie _lr_retry_request should be cleared after the ajax request?

Actual results

  1. The submodules get disrupted and throws an error in console
@dgirardi
Copy link
Collaborator

I cannot reproduce this part:

This disrupts the process, preventing all subsequent submodules from refreshing their IDs.

The issue as I see it is slightly different - the second call to pbjs.refreshUserIds() causes the promise returned by the first call to be rejected (and if the error is not handled, the browser will complain about an unhandled rejection). With #12246, it should resolve to the user IDs as refreshed by the second (or subsequent) call, when all submodules complete their refresh.

However it should not affect other ID modules (and from my testing it doesn't) - if identityLink never comes back with an ID, the promise(s) never resolve, but pbjs.getUserIds() (and auctions) should eventually still see the refreshed IDs for the modules that behave. The error you see is the mechanism used internally to cancel pending refreshes before starting a new one, it was not supposed to "escape" out of the public API but it also doesn't (or shouldn't) mean that other ID modules were interdicted.

If it's okay to make more ajax requests, the cookie _lr_retry_request should be cleared after the ajax request?

I am not sure about why identityLink behaves the way that it does. @mamatic ?

@danielsao
Copy link
Contributor

I'm also unable to reproduce your issue @yagovelazquezfreestar. Not being able to refresh should not affect other id modules.

The logging of the _lr_retry_request cookie is used for internal analytics to monitor system health. This cookie should not be cleared, only updated.​

The getEnvelopeFromStorage function retrieves our identity envelope from storage, where it was saved by ATS.js.

@yagovelazquezfreestar
Copy link
Contributor Author

@danielsao @dgirardi Thanks for the fast response. You're right, it was how I was running the userRefresh function that was causing the disruptive issue.

The other problem still stands though because although #12246 will handle the rejection of the promise, it won't fix - a) We should see a message written saying that the request returned an empty id. b) The reason why is failing in the first place.

So I did post this other pull request here addressing it. Do you think it's reasonable?

Other than that, @danielsao I'm not seeing any Ats function on the window in our pages, does that means that there is something wrong in our implementation? Please, refer to our implementation above

@dgirardi
Copy link
Collaborator

The logging of the _lr_retry_request cookie is used for internal analytics to monitor system health. This cookie should not be cleared, only updated.​

Does that mean that when this if condition is false, no attempt should be made to fetch/generate an ID?

if (!configParams.notUse3P && !storage.getCookie('_lr_retry_request')) {
setRetryCookie();
utils.logInfo('identityLink: A 3P retrieval is attempted!');
setEnvelopeSource(false);
ajax(url, callbacks, undefined, { method: 'GET', withCredentials: true });
}

if that's the case, the module should signal it as in #12249 - as it is currently it remains stuck waiting for an ID that'll never come.

@patmmccann
Copy link
Collaborator

hmmm #12249 was merged @danielsao . Are you okay with that?

@patmmccann patmmccann moved this from Triage to PR submitted in Prebid.js Tactical Issues table Oct 7, 2024
@github-project-automation github-project-automation bot moved this from PR submitted to Done in Prebid.js Tactical Issues table Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants