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

window is undefined when trying to run in worker file on browser #658

Closed
rashyad opened this issue Dec 10, 2023 · 8 comments · Fixed by #660
Closed

window is undefined when trying to run in worker file on browser #658

rashyad opened this issue Dec 10, 2023 · 8 comments · Fixed by #660
Labels

Comments

@rashyad
Copy link
Contributor

rashyad commented Dec 10, 2023

Describe the bug
The bug is regarding the window global object that is available when running in browser. However, when trying to run the upload process in a worker thread on client side, the window object is not defined in the worker global scope.

In the file /lib/browser/index.js, line 31 - 33, the code uses the window global object to check for XMLHttpRequest and Blob to determine the isSupported property.

image

Running this in worker will return window is not defined since the code assume that window is available. I understand the thoughts here since its supposed to be running in browser. I supposed we should put a check instead of assuming that window is available.

To Reproduce

  1. Run any web server of your choice, apache web server, node http-server etc so you can serve html and js files.
  2. Create two js files, one as the worker file and another one as the parent to init the worker using new Worker()
  3. Try to run the tus.Upload in the worker file and you will get the window not defined error in browser logs. You can copy the upload code part from /demos/browser/demo.js

Expected behavior
The upload should work on both the main thread and worker thread on browser. I supposed we should not be assuming that window would always be defined or something..

Just to add,
I have it working locally where I modified the file /lib/browser/index.js like so,
image

Built the code and it works fine. The upload works on both the main thread and worker thread.
However, this might not be the best fix.

@rashyad rashyad added the bug label Dec 10, 2023
@Acconut
Copy link
Member

Acconut commented Dec 11, 2023

Thanks for the report! You are right that window does not exist in the environment for Web Workers. Maybe we should use self instead for detecting support: https://developer.mozilla.org/en-US/docs/Web/API/Window/self and https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope/self

The following code could be a replacement without depending on window, although I didn't test it yet:

const isSupported = typeof self !== undefined && 'XMLHttpRequest' in self && 'Blob' in self && typeof Blob.prototype.slice === 'function'

Could you try it out and open a PR if it works?

@rashyad
Copy link
Contributor Author

rashyad commented Dec 11, 2023

Tested your code and it works fine with worker. The test-node and test-puppeteer run just fine.
However, I did not manage to create a specific demo for worker or a specific test for worker.

Here is my PR #660

@Acconut Acconut linked a pull request Dec 11, 2023 that will close this issue
@rashyad
Copy link
Contributor Author

rashyad commented Dec 13, 2023

May I know the status of this? Anything that I need to do on my side?

@Acconut
Copy link
Member

Acconut commented Dec 13, 2023

I didn't have the time yet to try it out but I plan on doing so in the next few days

@jtag05
Copy link

jtag05 commented Dec 17, 2023

I would also love to get this merged if it would solve this issue. If any additional help is required, I'm happy to lend a hand.

@Acconut
Copy link
Member

Acconut commented Dec 21, 2023

I added a few more commits to your PR and merged it. Can you please test the latest main branch from GitHub and let me know if it works? If so, I can release a new version.

@rashyad
Copy link
Contributor Author

rashyad commented Dec 21, 2023

Tested the main branch locally and works fine using with worker

@Acconut
Copy link
Member

Acconut commented Dec 22, 2023

Thanks for the confirmation! I just published v4.0.1, which includes this patch.

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

Successfully merging a pull request may close this issue.

3 participants