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

Error happens due to janus.js file #2529

Closed
Marvelanda opened this issue Jan 23, 2021 · 12 comments
Closed

Error happens due to janus.js file #2529

Marvelanda opened this issue Jan 23, 2021 · 12 comments

Comments

@Marvelanda
Copy link

Hi!

The error occurs due to the following issue in the janus.js file:

TypeError: Cannot read property 'browser' of undefined
350 line:
if(Janus.webRTCAdapter.browserDetails.browser === 'safari' &&
Janus.webRTCAdapter.browserDetails.version >= 605)

Previously everything work perfectly, tried to open the project on another devices, re-install dependencies and janus-gateway, but the problem remains.

Thank you for the answer in advance!

@lminiero
Copy link
Member

My guess is that this is fixed in #2531?

@Marvelanda
Copy link
Author

Marvelanda commented Jan 24, 2021

Unfortunately it seems that error appeared after this fix, before everything worked good.

@promerufon
Copy link

promerufon commented Jan 25, 2021

I had a problem with webrtc-adapter version 7.4.0.
The solution was as follows.
npm i --save [email protected]

There seems to be another reason for the solution.
Could not be reproduced.

There seems to be a problem with the default branch.
I was able to solve it by the following procedure.

  1. npm uninstall janus-gateway
  2. mkdir ~/janus && cd ~/janus && git clone https://github.com/meetecho/janus-gateway.git -b v0.10.9
  3. npm install ~/janus/janus-gateway

@lminiero
Copy link
Member

I was able to solve it by the following procedure.

That's not a fix. That's saying "I checked out an old branch", and so doesn't really help with the issue.
Unfortunately I never use the npm stuff so I can't help there. You may want to comment on the PR I referenced, and ask why you're still encountering the issue there.

@chrisuehlinger
Copy link

I just started running into this too. I'm gonna do the workaround for now, but in the interest of getting this solved, I took a look at the different versions of webrtc-adapter to see if something weird is going on. Here's what I found:

[email protected] (what I get when I put "janus-gateway": "https://github.com/meetecho/janus-gateway.git#0.10.9" in my package.json) has its "main" file set to "./src/js/adapter_core.js" in its package.json, and that file exports using module.exports = adapterFactory({window: global.window});

In the package.json for [email protected] (the new version from that PR), its "main" file is set to "./dist/adapter_core.js" which exports using exports.default = adapter;. That's probably the source of the problem (that they're using different module systems), and the solution may involve changing the guidance around how to configure webpack.

@chrisuehlinger
Copy link

Sorry for the commentspam, but I've got the fix. Update the webpack plugin config example in the "Using janus.js as JavaScript module" docs to the following:

const webpack = require('webpack');

module.exports = {
  plugins: [
    // janus.js does not use 'import' to access to the functionality of webrtc-adapter,
    // instead it expects a global object called 'adapter' for that.
    // Let's make that object available.
    new webpack.ProvidePlugin({ adapter: ['webrtc-adapter', 'default'] })
  ]
}

Getting the CLA approved by my new job might be a bit complicated, but if anyone whos already CLA'd can drop in this one-line docs change, it'll fix this issue.

@lminiero
Copy link
Member

Thanks for the feedback @chrisuehlinger! From what you're saying, it seems reasonable to revert the adapter version to what it was before (6.4.0?) in the package.json file, and then explain in the docs that, if you want a higher version, this is what you have to do (your new text). I suspect most users don't bother to read the docs at all, so if we just update those, people will still say it doesn't work.

@chrisuehlinger
Copy link

I’m not sure what changed between webrtc-adapter v6 and v7, it could be that one of your contributors bumped the version because of something important that comes up when like, working with RTCRtpTransceivers or something else deep in the weeds (webrtc-adapter doesn’t have a lot of release notes). Or maybe not, and it’s fine. But with the fast changing WebRTC browser APIs, I imagine you wouldn’t want to fall behind on adapter versions.

I totally get the idea of stemming the tide of complaints by just downgrading the version, but I would also point out that anyone who is using Janus with webpack has to know about that doc, otherwise there’s no way they could’ve gotten it working in the first place (I personally don’t have any other deps that use webpack.ProvidePlugin).

@lminiero
Copy link
Member

Fair enough! I'll update the docs myself later this morning using your proposed text, then 👍 Thanks again!
PS: you'll briefly appear in my FOSDEM slides in a couple of weeks 🤭

@lminiero
Copy link
Member

Actually, the docs had already been updated, in #2519... I haven't regenerated them yet, which is why the online docs still show the old and deprecated syntax. I'll do that now, and close this issue.

@lminiero
Copy link
Member

@promerufon
Copy link

That's not a fix. That's saying "I checked out an old branch",

That's right. Sorry for the misunderstanding.

In @chrisuehlinger comment, I noticed a configuration issue, so I checked it and found a mistake.
mistake point : new webpack.ProvidePlugin({ adapter: 'webrtc-adapter' })

Thank you, @lminiero and @chrisuehlinger .

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

No branches or pull requests

4 participants