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

Load Breaks with Privacy Badger #554

Open
KeKs0r opened this issue Jul 26, 2022 · 13 comments
Open

Load Breaks with Privacy Badger #554

KeKs0r opened this issue Jul 26, 2022 · 13 comments

Comments

@KeKs0r
Copy link

KeKs0r commented Jul 26, 2022

It seems that Privacy Badger blocks the call to get the settings.
The problem is, that means that the load call throws.
And i have in my code the analytics as a var in the global space

  export const analytics = SEGMENT_TOKEN
            ? await AnalyticsBrowser.load({ writeKey: SEGMENT_TOKEN })
            : undefined

I am currently failing to wrap this call in a try catch. Because the error is async. So in order to "catch" it, I would need to make analytics a Promise instead of the AnalyticsBrowser.
Which then means, I have to make all calls to it async.

I think it would be beneficial to handle those kind of errors within the sdk, to prevent breaking user code.
This is the error that breaks my app:

return fetch(`${baseUrl}/v1/projects/${writeKey}/settings`)
.then((res) => res.json())
.catch((err) => {
console.warn('Failed to load settings', err)
throw err
})
}

@chrisradek
Copy link
Contributor

@KeKs0r Thanks for creating this issue!

Can you update the code to include a catch handler like this?

  export const analytics = SEGMENT_TOKEN
            ? await AnalyticsBrowser.load({ writeKey: SEGMENT_TOKEN }).catch(() => undefined)
            : undefined

That will prevent errors from breaking your code - instead undefined is returned if an error is encountered. If there's no error, then analytics gets returned the awaited value of load.

AnalyticsBrowser.load() actually returns a tuple when you await it so you probably want something like:

  export const analytics = SEGMENT_TOKEN
            ? await AnalyticsBrowser.load({ writeKey: SEGMENT_TOKEN }).then(([analytics]) => analytics).catch(() => undefined)
            : undefined

@silesky
Copy link
Contributor

silesky commented Jul 27, 2022

Seems like a potentially common issue with blockers; we might want to just noop this kind of fetch error by default. I imagine most Segment users would prefer that this library avoid throwing wherever possible, especially when it’s due to something like this.

@KeKs0r
Copy link
Author

KeKs0r commented Jul 27, 2022

@chrisradek
this is actually what I tried to do.
But adding a "then" or "catch" on the load call, will turn it into a promise. And I can't call analytics.track() anymore. I would need to change my whole codebase to a (await analyics).track(). And currently all these calls are also in syncronous currently.

@silesky
I would agree that this would be the preferred behaviour for me.
If a user does not want to be tracked: don't break the application or make me handle it, but rather don't do anything, since its what the user wants anyways.

@silesky
Copy link
Contributor

silesky commented Jul 27, 2022

@KeKs0r ... did you try using the synchronous API?

 export const analytics = SEGMENT_TOKEN
            ? AnalyticsBrowser.load({ writeKey: SEGMENT_TOKEN })
            : undefined
            
analytics.track('foo')

It should log load errors, but should not throw.

edit: I added a test to confirm this behavior https://github.com/segmentio/analytics-next/pull/557/files#diff-47514c4c862cd4b0072c15f4bb644190a93539c5c089f647fd418239b4f928a4R189

@KeKs0r
Copy link
Author

KeKs0r commented Jul 29, 2022

@silesky yes I ran into this in production. Also updated to the newest version of this library. Only way to get around it in my codebase was to remove the throw part in the catch.

Maybe it also depends on the bundling, since my analytics is defined on the top level of a module. So depending on how a bundler might wrap the file import, it potentially throws on a top level.

@prsnca
Copy link

prsnca commented Feb 1, 2023

@silesky My client breaks with an adblocker is present (uBlock origin in my case) - it blocks our Hubspot integration (The URL is from this format https://js.hs-analytics.net/analytics/XXXXX/XXX.js
The weird thing is that we do not even get into the catch clause on failure, the application hangs and does not continue to load. We are doing it synchronously using await since we want to start sending events right away.

Any suggestions here?

Thanks!

@silesky
Copy link
Contributor

silesky commented Feb 1, 2023

If we're still just talking about catching any initial load errors and doing something with them (besides console logging), the new, documented way is:

export const analytics = new AnalyticsBrowser();
analytics
  .load({ writeKey: "MY_WRITE_KEY" })
  .catch((err) => ...);

Note: this catch clause will not handle device mode script blocked errors -- the only adblocking errors that it should handle would be if the settings CDN gets blocked.

I am interested in this adblocker case -- some questions:

@prsnca

  1. it's only blocking the hubspot script, but not anything else (typically segment's cdn is the thing that gets blocked) ?

  2. When the hubspot script gets blocked, does it take down your entire application / page; or, is it just that the analytics client does not send any events to Segment, but things work as usual?

  3. Can you post your analytics initialization code?

@prsnca
Copy link

prsnca commented Feb 1, 2023

We have tried to upgrade the client to the latest version but still could not catch the failure under that catch clause which is very weird...
this is how we do it today - causes the application to hang and the failure does not get to the catch part... also tried to debug it but couldn't get to the bottom of the error.

try {
            const [analytics] = await AnalyticsBrowser.load(
                {
                    writeKey: segmentWriteKey,
                    cdnURL: SEGMENT_PROXY_CDN,
                },
                {
                    initialPageview: false,
                    integrations: {
                        'Segment.io': { apiHost: SEGMENT_PROXY_API },
                    }
                }
            );
            this.segmentClient = analytics;
        } catch (err: any) {
            console.log('Error initializing Segment', err);
            return;
        }

upgraded to this, and still can't get to the catch clause on failure:

try {
    AnalyticsBrowser.load({
                    writeKey: segmentWriteKey,
                    cdnURL: SEGMENT_PROXY_CDN,
                },
                {
                    initialPageview: false,
                    integrations: {
                        'Segment.io': { apiHost: SEGMENT_PROXY_API },
                    }
                })
                .then(async ([analytics, _context]: [Analytics, Context]) => {
                    this.segmentClient = analytics;
                })
                .catch(() => undefined);
        } catch (err: any) {
            console.log('Error initializing Segment', err);
            return;
        }
  1. it's blocking a lot of scripts, but blocking this specific one causes the failure (I played with the adblocker's allowlist)
  2. it just hangs... we're on await there and it never returns - so the application does not continue to start up
  3. ^^

Thanks @silesky!

@davidgonor1408
Copy link

Hey @silesky, having the same issue as @prsnca.
Do you have any suggestions?
Thanks

@erickarnis-tb
Copy link

erickarnis-tb commented Nov 27, 2023

Try this

const hasKey = Boolean(import.meta.env.SEGMENT_TOKEN)

export const track = async (...args: Parameters<typeof analytics.track>) => {
  if (hasKey) {
    await analytics.track(...args)
  }
}

@Skwai
Copy link

Skwai commented Mar 15, 2024

I've just encountered this.

The problem I had was that Segment would load but fail to fetch settings (request was blocked by browser).

This would mean that analytics.ready would never resolve.

To solve I had to wrap analytics.user() calls in a timeout closure to handle cases where Segment will have loaded but ready will never have resolved.

export const withTimeout = <T>(promise: () => Promise<T>, timeout: number): Promise<T | undefined> => {
  return new Promise<T>((resolve, reject) => {
    const id = setTimeout(() => {
      reject(new Error('Timed out'))
    }, timeout)

    promise()
      .then((result) => {
        clearTimeout(id)
        resolve(result)
      })
      .catch((error) => {
        clearTimeout(id)
        reject(error)
      })
  })
}

const user = await withTimeout(() => analytics.user(), 2_000)

if (user) {
  console.log(user.anonymousId())
}

@georgebutter
Copy link

georgebutter commented Mar 20, 2024

I have Segment running via a proxy and it's working great based on the instructions here. https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/custom-proxy/

However, the destinations I have set up (for example Mixpanel) also load a cdn (http://cdn.mxpnl.com/libs/mixpanel-2-latest.min.js), which is not proxied and is therefore blocked. Is there a way to use the integrations options to also proxy these requests?

@silesky
Copy link
Contributor

silesky commented Mar 20, 2024

However, the destinations I have set up (for example Mixpanel) also load a cdn (http://cdn.mxpnl.com/libs/mixpanel-2-latest.min.js), which is not proxied and is therefore blocked. Is there a way to use the integrations options to also proxy these requests?

@georgebutter -- analytics.js is just a loader for destinations.

Unless there's some mixpanel global you can modify to update that config, that option would be something that would need to be supported on the action side and likely passed to mixpanel via instantiation. Currently, there's no option for configuring api host / proxy options in mixpanel via segment.

You could make a support request or a contribution to https://github.com/segmentio/action-destinations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants