launchdarkly / js-client-sdk

LaunchDarkly Client-side SDK for Browser JavaScript
Other
109 stars 62 forks source link

Event streaming in iOS standalone browser #258

Closed skcarnes closed 1 year ago

skcarnes commented 1 year ago

Describe the bug The LDClient is failing to establish a connection to stream flag updates

To reproduce Subscribe to the ldClient.on('change') events

Expected behavior Updates to flags should be reflected in the client

Logs Screen Shot 2022-10-14 at 9 56 03 AM

SDK version 2.20.0

Language version, developer tools Angular 14 PWA

OS/platform iOS Safari standalone browser (PWA)

Additional context So we are only seeing this issue if our web app is running as a PWA or "Add to Home Screen" on an iOS device. This works on all other platforms we've tested. It appears that there is something wrong with the native safari EventSource establishing a connection with LD. We were able to replace the native EventSource code with the polyfill provided in the docs, but this seems wrong as a long term solution.

kinyoklion commented 1 year ago

Hello @skcarnes,

I cannot tell from your log screenshot, but considering there isn't a status, is this failing a preflight check like CORS? I think there should be a corresponding console log entry.

I will see if I can replicate the problem.

Filed internally as 173125

Thank you, Ryan

kinyoklion commented 1 year ago

Hello @skcarnes,

I setup a test using an iPhone 13 with iOS 16.0.2 and a basic PWA using a manifest with the SDK configured for streaming. I added it to the home screen and it was able to establish a connection to the clientstream.

Would it be possible for you to produce a minimum reproduction case?

Thanks, Ryan

skcarnes commented 1 year ago

@kinyoklion Thanks for the quick response.

This is the only console log we are getting

image

Here's an example of what we're seeing in our production env -> https://skcarnes.github.io/

This works for other platforms, but I'm still seeing issues on iOS "Add to Home Screen"?

kinyoklion commented 1 year ago

Hello @skcarnes,

Thank you for the example. I do see the same issue with it, and am trying to track down what may be causing the difference between it and the very basic application I am working from.

My feeling is that it is CORS related, but unfortunately the safari error messages are lacking in any detail.

Thanks, Ryan

kinyoklion commented 1 year ago

Hello @skcarnes,

Thank you for the example. I do see the same issue with it, and am trying to track down what may be causing the difference between it and the very basic application I am working from.

My feeling is that it is CORS related, but unfortunately the safari error messages are lacking in any detail.

Thanks, Ryan

Following up to this, I am curious if there is some service worker interaction here that is at the root of the problem.

kinyoklion commented 1 year ago

I am not terribly familiar with how angular configures service workers, but I do see that the service worker is satisfying requests to LaunchDarkly. Which is not what it should be doing, and it is likely causing problems. (Potentially a bug in safari with the scoping of the service worker?)

Screen Shot 2022-10-25 at 10 30 05 AM
skcarnes commented 1 year ago

So as it turns out this is a problem with the angular service worker and we're seeing issues across all safari browsers. I'm not sure how we missed this... Angular does provide a way to bypass the service worker, but you have to include additional headers or a query param. Do you guys have a way of including one of those to your network calls?

kinyoklion commented 1 year ago

Hey @skcarnes,

The SDK has a requestHeaderTransform option.

    /**
     * A transform function for dynamic configuration of HTTP headers.
     *
     * This method will run last in the header generation sequence, so the function should have
     * all system generated headers in case those also need to be modified.
     */
    requestHeaderTransform?: (headers: Map<string, string>) => Map<string, string>;

So you can add in the headers you need.

Thanks, Ryan

kinyoklion commented 1 year ago

So as it turns out this is a problem with the angular service worker and we're seeing issues across all safari browsers. I'm not sure how we missed this... Angular does provide a way to bypass the service worker, but you have to include additional headers or a query param. Do you guys have a way of including one of those to your network calls?

One thing I noticed in testing with a local sample was that the behavior didn't happen on the very first page load, because the service workers likely weren't registered yet. So I could see how it may take some time to notice.

skcarnes commented 1 year ago

It looks like we were able to bypass our service worker for the initial call, but the attempt to setup the stream is still failing and doesn't have the included headers. It doesn't look like there's a way to add headers to EventSource.

I believe we would be able to get around this if there was a way to add query params to the EventSource. I see there's a streamUrl, but it looks like that's just the baseUrl. Any other way of doing this? If not, we'll just have to look further into implementing a custom service worker. Thanks!

Screen Shot 2022-10-26 at 1 41 26 PM

kinyoklion commented 1 year ago

It looks like we were able to bypass our service worker for the initial call, but the attempt to setup the stream is still failing and doesn't have the included headers. It doesn't look like there's a way to add headers to EventSource.

I believe we would be able to get around this if there was a way to add query params to the EventSource. I see there's a streamUrl, but it looks like that's just the baseUrl. Any other way of doing this? If not, we'll just have to look further into implementing a custom service worker. Thanks!

Screen Shot 2022-10-26 at 1 41 26 PM

Ah yes, sorry I forgot about that. We support headers when using the EventSource polyfill (we provide one), but the event source implementation in browsers doesn't support headers.

It does really feel like there is a bug with either angular or safari here, because the default scope of a service worker really should only includes things relative to the source adding the service worker. Not intercepting API calls to other domains.

skcarnes commented 1 year ago

I agree. We ended up wrapping the angular generated service worker with our own to handle this issue. If anyone else is running into something similar you can check out this https://github.com/angular/angular/issues/36403#issuecomment-608695735.

Thank you guys so much for your time and your help.