twilio / twilio-video.js

Twilio’s Programmable Video JavaScript SDK
https://www.twilio.com/docs/video/javascript
Other
571 stars 217 forks source link

Crashes on missing `process.env` #1746

Closed Dosant closed 2 years ago

Dosant commented 2 years ago

Actual behavior:

twilio-video.js since 2.21.0, 2.21.1 expects process.env to be available globally. Otherwise it fails with an exception during import.

Expected behavior:

process.env shouldn't be required in browser for twilio-video to work or should be documented as a requirement

Apparently the root cause is this code:

https://github.com/twilio/twilio-video.js/blob/352ebf3e1d85aacdf4a888ba8a99e4ab37465131/lib/webrtc/rtcpeerconnection/chrome.js#L6

Where it imports and bundle node module util that assumes execution in node environment. This fails in user-land runtime if an app's bundler doesn't supply necessary polyfills.

And the crash in the browser happens on this line: https://github.com/browserify/node-util/blob/ad9dbe0df07655a38f0f8fc8feaeccb965d207e7/util.js#L109-L116

Software versions:

charliesantos commented 2 years ago

Hey @Dosant are you seeing this on Angular only?

Dosant commented 2 years ago

@charliesantos, I haven't tried to reproduce outside my app which is angular. I believe it is possible to come up with a minimal webpack config where this would fail without angular

charliesantos commented 2 years ago

@Dosant ok no problem. We are in progress of investigating this issue.

PikaJoyce commented 2 years ago

Hi @Dosant,

I'm currently investigating this issue and I was wondering if you could provide more details on how to repro this. Perhaps a repo or a config set up may be useful.

I've currently tried to reproduce this using Angular 12 using a fresh project (created using ng new) and I'm not seeing any errors related to the one you've mentioned.

Thank you! Joyce

Dosant commented 2 years ago

Hi @PikaJoyce,

Here is the repro starting from ng new: https://github.com/Dosant/my-app Here is the commit on top of ng new: https://github.com/Dosant/my-app/commit/7d6896de58ca863d35f567b07a6e8b5cb1c7049b

Starting the app with ng serve I get (coming from twillio on the lines I mentioned in the issue description) :

[Error] ReferenceError: Can't find variable: process
    (anonymous function) (vendor.js:34936)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (vendor.js:33181)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (vendor.js:33155)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (vendor.js:32238)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (vendor.js:5773)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (vendor.js:6458)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (main.js:14:90)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (main.js:323:92)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (main.js:377:93)
    __webpack_require__ (runtime.js:23)
    (anonymous function) (runtime.js:57)
    (anonymous function) (main.js:396)
    webpackJsonpCallback (runtime.js:145)
    webpackJsonpCallback
    Module Code (main.js:2)
    evaluate
    moduleEvaluation
    (anonymous function)
    promiseReactionJob

I believe this is not angular build specific issue, but can be also reproduced with a simpler webpack config (didn't try)

PikaJoyce commented 2 years ago

Hey @Dosant!

Thank you so much for that info. So I cloned your repository and actually ran it. I still wasn't able to repro this. The only other thing I can think of is perhaps it's a difference in node versions? I'd love to be able to dig a bit deeper if it's possible. I'll include a screen shot of what I get when running start on your repo.

image

Dosant commented 2 years ago

@PikaJoyce, the error occurs in runtime, so you have to open the app localhost:4200 in a browser to see it fail. Have you tried that? (sorry if that wasn't clear from the issue description)

rbclark commented 2 years ago

I am seeing this using esbuild as well. The application compiles successfully however I receive a Uncaught ReferenceError: process is not defined in the browser on execution. All I am doing is including twilio with a call to const { connect, Participant } = require('twilio-video');, and then building with esbuild and then trying to load the application (which uses twilio-video) in the browser.

PikaJoyce commented 2 years ago

Hi @Dosant

I apologize, it totally skipped my eyes initially. I see the issue now! Thank you for pointing that out and please don't apologize!

@rbclark, thank you for your comment as well. Hopefully we get this resolved soon.

This work is currently being tracked by VIDEO-9282.

Best, Joyce

PikaJoyce commented 2 years ago

Hi again @Dosant @rbclark

In the meantime, here's a workaround to unblock you while we continue to work on a more permanent solution:

  1. In your project, run npm i process.

  2. Open src/polyfills.ts

  3. Add the below lines inside of polyfills.ts

    (window as any).global = window;
    (window as any).process = { env: { DEBUG: undefined }, }

While this is not a permanent solution, it gets the app to run error free for the time being. Thank you for your patience. We hope to get this issue resolved soon.

Best, Joyce

rbclark commented 2 years ago

Thank you @PikaJoyce! I've been using @esbuild-plugins/node-modules-polyfill for now in order to polyfill the required module for now.

davbrito commented 2 years ago

This problem surfaces also when using vite. Since vite doesn't use process.env but import.meta.env.

hi-reeve commented 2 years ago

This problem surfaces also when using vite. Since vite doesn't use process.env but import.meta.env.

got the same issue with vite. hope this be priority

PikaJoyce commented 2 years ago

Hey folks,

A quick update! This PR was recently merged and will go through several rounds of testing before a full release. Looking forward to getting this into everyones hands!

Best, Joyce

PikaJoyce commented 2 years ago

Hi everyone,

The fix for this issue has been resolved in the most recent release. Please check it out and let me know if there are any further issues. I will now be closing this issue, but please do feel free to continue the conversation here or open another issue with us.

Best, Joyce