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

Webex SDK 3.0.0 not compatible with any version of NodeJS #3510

Open
Tiuipuv opened this issue Apr 2, 2024 · 18 comments
Open

Webex SDK 3.0.0 not compatible with any version of NodeJS #3510

Tiuipuv opened this issue Apr 2, 2024 · 18 comments
Assignees
Labels

Comments

@Tiuipuv
Copy link

Tiuipuv commented Apr 2, 2024

Describe the bug
The following script fails to initialize in NodeJS:

import webex from 'webex';

webex.init({credentials: '<valid credentials were provided here>'});

Yielding the following error:

ReferenceError: window is not defined
    at Object.<anonymous> ([...]\node_modules\@webex\plugin-meetings\node_modules\@webex\internal-media-core\dist\cjs\index.js:2065:29024)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Module.load (node:internal/modules/cjs/loader:1203:32)
    at Module._load (node:internal/modules/cjs/loader:1019:12)
    at Module.require (node:internal/modules/cjs/loader:1231:19)
    at require (node:internal/modules/helpers:177:18)
    at Object.<anonymous> ([...]\node_modules\@webex\plugin-meetings\dist\meetings\index.js:35:26)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)

This has been tested with nodejs v12, v14, v16, v18, and v20.
package.json:

{
  "name": "test_webex",
  "type": "module",
  [...]
  "dependencies": {
    "webex": "^3.0.0"
  }
}

This error is produced for both ESM modules and CommonJS scripts.

To Reproduce
Steps to reproduce the behavior:

  1. create a blank npm project using npm init
  2. install webex sdk with npm install webex
  3. create index.js, with the above code (enter valid bot id)
  4. run with node index.js
  5. See error

Expected behavior
Webex SDK successfully imports into script.

Platform (please complete the following information):

  • OS: Windows 10 Pro 22H2
  • Browser: N/A (NodeJS)
  • Version: v12.22.12, v14.21.3, v16.20.2, v18.20.0, v20.12.0
  • Device Type: Desktop
@Tiuipuv Tiuipuv added the bug Bug label Apr 2, 2024
@valgaze
Copy link

valgaze commented Apr 3, 2024

+1 was about to file an issue as well

Confirm failure in node 21

image

@valgaze
Copy link

valgaze commented Apr 6, 2024

@Tiuipuv

Two things:

  1. This is happening because code modifies globalThis.navigator which in node 21, the navigator object is special
  1. Regardless the WebEx sdk is NOT isn't yet supporting newer versions of node: [Feb 2024] TypeError: Cannot set property navigator of #<Object> which has only a getter #3393 (comment)

Regarding item 1:

  • In the dist code (@webex/internal-media-core/dist/cjs/index.js) there's a rather long ternary expression to set commonjsGlobal:
var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};

When unpacked it looks like this (and 1st condition matches in node21):

let commonjsGlobal;

if (typeof globalThis !== 'undefined') {
  commonjsGlobal = globalThis;
} else if (typeof window !== 'undefined') {
  commonjsGlobal = window;
} else if (typeof global !== 'undefined') {
  commonjsGlobal = global;
} else if (typeof self !== 'undefined') {
  commonjsGlobal = self;
} else {
  commonjsGlobal = {};
}

Later on this file mock some methods on a phony commonjsGlobal.navigator (which is ultimately globalThis.navigator):

commonjsGlobal.navigator = {
          userAgent: browserFakeUserAgent,
          getUserMedia: function getUserMedia() {}
      };

Regarding item 2: The lack of updates is a bummer, however, there might be fixes coming from node patches itself. The now-term solution is back down a version

cc: @arun3528 @jbenyovs

@Tiuipuv
Copy link
Author

Tiuipuv commented Apr 6, 2024

@valgaze

Thank you for your in depth analysis. However, I am not sure downgrading to any version of node works. I noticed this navigator error on v20+, but all versions below 20 all the way down to 12 we're also failing to startup as well (tested using volta).

When you proposed the 'now-term' solution of downgrading, were you referring to downgrading node, or the package itself back to 2.60.2?

@valgaze
Copy link

valgaze commented Apr 6, 2024

@Tiuipuv I'm realizing your error is slightly different:

Your issue: issues about window object in many versions of node as you stated

Other issue: globalThis.navigator only occurs in node 21+

They seem related but curious what the maintainers of this library think, but maybe it's smart to split this into separate issues

@samtsai
Copy link

samtsai commented Apr 8, 2024

#2242 another issue exists for window is not defined

@Tiuipuv Tiuipuv changed the title Webex SDK 3.0.0 not compatible with all version of NodeJS Webex SDK 3.0.0 not compatible with any version of NodeJS Apr 10, 2024
@sreenara sreenara self-assigned this Apr 22, 2024
@sreenara
Copy link
Contributor

@Tiuipuv thanks for reporting this issue. We have also observed this internally and are currently looking into this. Will provide updates once we have a chance to identify where the root cause is.

Just another note that at this time, we haven't tested all the features of the JS SDK with Node 18 and above. Our applications are running Node 16 and consuming the SDK.

@sreenara
Copy link
Contributor

Tagging @ricardocasares as well from #2242

@sreenara
Copy link
Contributor

sreenara commented Apr 23, 2024

Hey folks, I did some testing and here's a workaround you'll can use to continue testing. I tested this locally and was able to run it on Node 16, 18 and 20 to establish a successful registration with webex.meetings

global.window = {};
global.window.location = {};
global.window.navigator = {
    userAgent: 'test'
};
global.window.setTimeout = setTimeout;
// global.Webex = {};

const Webex = require('webex');

const webex = (window.webex = Webex.init({
    credentials: {
      access_token: 'token'
    }
  }));

webex.once('ready', () => {
    if (webex.canAuthorize) {
        console.log('webex is authorized');
        webex.meetings.register().then(() => {console.log('registered to meetings', webex.internal.device.userId);})
    }
});

This requirement of window object seems to have come from one of the dependencies and we are investigating it. It will take us a few days to get to the root cause.

@arun3528
Copy link
Collaborator

@sreenara there is a need to discuss about how to ship non browser package efficiently + testing

@Tiuipuv
Copy link
Author

Tiuipuv commented Apr 29, 2024

Thank you for the quick movement on this issue. Looking forward to a solution. The above workaround, with setting global.window.* to browser simulated values, is a great workaround for a low dependency project. However, this is dangerous to do in an environment with tons of other packages that use the existence of certain global.window.* to know if they are running in the browser.

@sreenara sreenara added feature and removed bug Bug labels May 13, 2024
@sreenara
Copy link
Contributor

Marking this as a feature and not a bug. On closer inspection of the code, what we are observing here is expected behavior. The goal now would be to find out whether we can publish a variant of webex.min.js that will NOT require the window object. In this variant,

  • Not all plugins in the webex-js-sdk will be supported in NodeJS.
  • Functionality such as media handling will not be supported as the Streams classes need a browser to acquire the list of devices and use them.

@enyineer
Copy link

enyineer commented Aug 2, 2024

Is this still worked on? No fix since 3 months.
I wouldn't have believed a commercial product to be so badly maintained. Suggesting to use EOL versions of Node just to use an SDK is not something I would expect from Cisco.
Huge Red Flags to avoid this product and rather use competitor products...

@sreenara
Copy link
Contributor

sreenara commented Aug 6, 2024

Is this still worked on? No fix since 3 months. I wouldn't have believed a commercial product to be so badly maintained. Suggesting to use EOL versions of Node just to use an SDK is not something I would expect from Cisco. Huge Red Flags to avoid this product and rather use competitor products...

@enyineer yes, we are planning to add support for NodeJS. The team will get to this in a month or so.

Just another note that at this time, we haven't tested all the features of the JS SDK with Node 18 and above. Our applications are running Node 16 and consuming the SDK.

Suggesting to use EOL versions of Node just to use an SDK

The Webex JS SDK can be compiled with Node 20 without any issues.
We've also done some testing on Node 20. Meeting and calling features will work if the SDK is consumed on the client side.

@johnwfrancis
Copy link

@sreenara until it's fixed I've unpublished our node.js doc since the instructions don't actually work.

@sreenara
Copy link
Contributor

sreenara commented Aug 25, 2024

An update on this one. Developers can use the jsdom-global package in their applications to avoid hitting the problem while initializing the webex object. This works with NodeJS 20.
Example of script:

require('jsdom-global')();
const Webex = require('webex');

const webex = (window.webex = Webex.init({
    credentials: {
      access_token: 'token'
    }
  }));

webex.once('ready', () => {
    if (webex.canAuthorize) {
        console.log('webex is authorized');
        webex.rooms.create({
            title: 'Test Space from NodeJS'
        })
        .then(function(room) {
            console.log('room object', room);
            webex.messages.create({
                    text: 'Hello World!',
                    roomId: room.id
                });
        })
        // Make sure to log errors in case something goes wrong.
        .catch(function(reason) {
        console.error(reason);
        process.exit(1);
        });
    }
});

@baranork
Copy link

Are there any news if the Webex SDK 3.0.0 will work on Node 22 when it enters Long Term Support on October?

@sreenara
Copy link
Contributor

Are there any news if the Webex SDK 3.0.0 will work on Node 22 when it enters Long Term Support on October?

@baranork We haven't tested it on Node 22 yet. Are there any differences between Node 20 and 22 which you feel will be a concern?

@Tiuipuv
Copy link
Author

Tiuipuv commented Aug 30, 2024

So far my testing shows that v22 should work for basic tasks, such as room info, post message, etc. However, it is worth noting with the later versions of webex js sdk (v3.1.X+), we often get the following errors:

failed to execute Logger#log TypeError: Cannot read properties of undefined (reading 'logger')
    at CallDiagnosticMetrics.get (C:\Users\h35529\Downloads\webex-sdk-test\node_modules\@webex\webex-core\dist\lib\stateless-webex-plugin.js:79:25)
    at assignValue (c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2514:28)
    at c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2733:9
    at arrayEach (c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:530:11)
    at baseClone (c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2727:7)
    at c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2733:34
    at arrayEach (c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:530:11)
    at baseClone (c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2727:7)
    at c:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:2733:34
    at arrayEach (C:\Users\h35529\Downloads\webex-sdk-test\node_modules\lodash\lodash.js:530:11) {stack: 'TypeError: Cannot read properties of undefine…test\\node_modules\\lodash\\lodash.js:530:11)', message: "Cannot read properties of undefined (reading 'logger')"}

This is including the workaround of adding require('jsdom-global')();.

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

No branches or pull requests

8 participants