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

Jump to bottom

Service Worker overwriting window object causes crash #2538

Closed
timostark opened this issue Jan 2, 2021 · 7 comments · Fixed by #2533
Closed

Service Worker overwriting window object causes crash #2538

timostark opened this issue Jan 2, 2021 · 7 comments · Fixed by #2533
Assignees
@LavrovArtem
Labels
AREA: client FREQUENCY: level 2 SYSTEM: workers TYPE: bug
Milestone
Sprint #73

Comments

@timostark
Copy link

timostark commented Jan 2, 2021 •
edited

Hi All,

First i am very sorry, for the bad bug report. The scenario is happening specifically with SAP Analytics Cloud (Major Analytics solution by SAP). Everything here is of course properitary. I hope the technical details and possible solution are good enough to understand the issue.

Generally: Cloud Analytics is heavily relying on service workers. This seems to cause issues.

Summary

Crash during startup, in case a service worker is (partly) filling the window object.

Technical Reason for crash

Native Methods nodeFirstChildGetter is undefined when using XHR requests in service-worker.
grafik

As a service worker does not have a window.Node/HTMLElement object, the resolve method shouldn't be called after all. The code for this logic is already available - The problem seems to be that the caller (resolve) is not detecting that it is inside a worker thread (see following screenshot).
grafik

The reason for this, is that the window object was partly overwritten (with DedicatedWorkerGlobalScope - so the actual "self" object). If I open the application directly, it shows the same behavior inside the worker thread. Therefore the window object is overwritten / defined by the service worker.
grafik

Now I am really not sure about the service worker specification. Is a service worker allowed to overwrite any window method? Adjusting the code to:
grafik
solves the issue and the application simply starts up..
Alternative (yes the ts-ignore is ofc bullshit):
grafik

What do you think? Is that a valid solution (maybe including Node and other window objects you are accessing..)

Thanks for any feedback,
Timo
@timostark timostark changed the title Service Worker overwriting window object causes issues Service Worker overwriting window object causes crash Jan 2, 2021
@timostark
Copy link
Author

Remark: I just found the causing code inside the Cloud Analytics application:
grafik

So yes - SAP is setting the window object inside the service worker on itself. Again I don't know if that is not allowed. Anyhow of course the local browser is accepting that (without warning), and the crash is only occuring with testcafe (cause is clear), so I would say yes..

@timostark timostark mentioned this issue Jan 3, 2021
Closed
@timostark
Copy link
Author

Additional issue (wont raise as new bug, as almost 100% identical). If the service-worker is overwriting "document", it causes a crash for the same reason due to "incorrect/not sufficient" service worker detection in: src\client\utils\destination-location.ts.
grafik

Error:
grafik

@Farfurix
Copy link
Contributor

Farfurix commented Jan 4, 2021

@timostark

Hello,

What do you think? Is that a valid solution (maybe including Node and other window objects you are accessing..)

I'm afraid we cannot say anything regarding this solution until we reproduce the issue ourselves. Could you please share a simple project (or a public URL)? We'll examine it and check for a suitable solution.

If your project is private, you can use our mailbox (support@devexpress.com). However, since your website sits behind a proxy and requires authentication, we will not be able to access it. Our policy prevents us from accessing a customer’s internal resources without a prior written approval from the entity that owns the server/web resource. If you want us to research the problem further, we’ll need access to the server/web resource. Please ask the website owner to send us (support@devexpress.com) a written confirmation. It must permit DevExpress personnel to remotely access the website and its internal resources for research/testing/and debugging purposes.

@timostark
Copy link
Author

timostark commented Jan 4, 2021 •
edited

@Farfurix : Uploaded a reproducible to https://lemokla.de/service_worker_hammerhead/index.html
Please note that those are two different issues, with a very similar root cause.

  1. Failure in Dedicated Worker for XMLHTTPRequest, in case document is overwritten
  2. Failure in Service Worker for fetch, in case window is overwritten.

Possible Code solution is maintained in the first post (using instanceof, instead of just checking the window object),

grafik

Code:
index.html

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
    <meta name="viewport" content="width=device-width">

    <title>Service worker scope</title>
  </head>

  <body>
    <h1>Service worker</h1>

    <section></section>

    <script src="app.js?v=13"></script>
  </body>
</html>

app.js

if ('serviceWorker' in navigator) {
    navigator.serviceWorker.register('./sw.js', { scope: ' /service_worker_hammerhead/ ' }).then(function (reg) {
        if (reg.installing) {
            console.log('Service worker installing');
        } else if (reg.waiting) {
            console.log('Service worker installed');
        } else if (reg.active) {
            console.log('Service worker active');
        }

    }).catch(function (error) {
        // registration failed
        console.log('Registration failed with ' + error);
    });
}
if (window.Worker) {
    const myWorker = new Worker("./worker.js?v=2");
}

sw.js

self.window = self;
self.document = self;

fetch('./index.html?fromSW=1').then(function (response) {
});

worker.js

self.window = self;
self.document = self;

var oReq = new XMLHttpRequest();
oReq.open("GET", "./index.html?fromWorker=2");
oReq.send();

@timostark
Copy link
Author

For me the issue is solved with the two following adjustments:

Dedicated Worker, for XMLHTTPRequest.open ( src\client\utils\destination-location.ts )
grafik

Service Worker with Fetch ( src\client\utils\global-context-info.ts )
grafik

@AlexKamaev
Copy link
Contributor

@timostark
Thank you for sharing your example. I was able to reproduce the issue. I confirm that everything works without TestCafe. The error occurs if I run the project with TestCafe or with the testcafe-hammerhead playground.
Thank you for your detailed research. It looks like you found the cause of the issue. However, we still need to discuss the issue with the team, so it can take some time. Please follow the updated in this thread.

@LavrovArtem LavrovArtem linked a pull request Jan 20, 2021 that will close this issue
The global context info calculated once per context #2533
Merged
@LavrovArtem
Copy link
Contributor

It fixed partially in #2533

@LavrovArtem LavrovArtem added this to the Sprint #73 milestone Jan 20, 2021
@LavrovArtem LavrovArtem self-assigned this Jan 20, 2021
LavrovArtem added a commit to LavrovArtem/testcafe-hammerhead that referenced this issue Jan 20, 2021
@LavrovArtem
fix Service Worker overwriting window object causes crash (close De…
0fef637 
…vExpress#2538)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@LavrovArtem LavrovArtem

Labels
AREA: client FREQUENCY: level 2 SYSTEM: workers TYPE: bug
Projects
None yet
Milestone
Sprint #73
Development

Successfully merging a pull request may close this issue.

The global context info calculated once per context LavrovArtem/testcafe-hammerhead
5 participants
@AlexKamaev @AndreyBelym @LavrovArtem @timostark @Farfurix