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

browser.js blows up inside _normalizeService when service has no fullname or txt #35

Open
johnspurlock opened this issue Nov 10, 2016 · 14 comments

Comments

@johnspurlock
Copy link

e.g. a service like the one below passed to _normalizeService will fail in two ways: normalizing the name (no service.fullname), and normalizing the txtRecord (no service.txtRecord)

{
  "addresses": [
    "192.168.1.100"
  ],
  "query": [],
  "type": [
    {
      "name": "homekit",
      "protocol": "tcp",
      "subtypes": []
    }
  ],
  "interfaceIndex": 3,
  "networkInterface": "pseudo multicast"
}
@MarshallOfSound
Copy link
Member

I assume your referencing the node-mdns-easy package.

https://github.com/GPMDP/node-mdns-easy/blob/master/src/browser.js#L67

@MarshallOfSound
Copy link
Member

Would you just expect the fullname to be blank if none is provided in the record?

(A blank fullname actually violates the MDNS spec but that's a different story)

@MarshallOfSound
Copy link
Member

/cc @jostrander I think you did this normalizing stuff, thoughts on how to handle?

@johnspurlock
Copy link
Author

That's right, browser.js = node_modules/node-mdns-easy/dist/browser.js

Would a sensible default be host + first service type or something? Not sure how fullname is used in your package, I just started kicking the tires tonight.

@MarshallOfSound
Copy link
Member

@johnspurlock I think we can just default it to the scanning type. We don't actually use fullname as far as I can see in this package. I think we used to but we replaced it with a text record.

https://github.com/GPMDP/electron-chromecast/blob/master/src/cast/index.js#L28

I don't know why we are even processing the record you posted though? It's type is homekit not cast?

@jostrander
Copy link
Member

Yeah I just made it normalized for our application, if we're missing something that could be used elsewhere by all means we can add a sensible default in.

@jostrander
Copy link
Member

Valid point though @MarshallOfSound that's not a valid google cast record.

@johnspurlock
Copy link
Author

I had other apps doing bonjour things at the time, I assume it was not explicitly requested by electron-chromecast, just observing another advertised service. Perhaps you can filter them out upstream somewhere?

@MarshallOfSound
Copy link
Member

@johnspurlock That would seem quite strange if it wasn't explicitly requested for node-mdns-easy to still receive it. Regardless if you want to submit a PR to node-mdns-easy with a sensible default I'll gladly accept it 👍

@johnspurlock
Copy link
Author

Ok thanks, I ended up using another library in the meantime - will look into a patch if I re-integrate this library.

@MarshallOfSound
Copy link
Member

another library

For chromecast integration? Can you link me through? 😄

@johnspurlock
Copy link
Author

Sure! Right now I'm just using castv2-client directly, which uses the same castv2 dependency, but no mdns abstraction or chrome api emulation.

@dskvr
Copy link

dskvr commented May 4, 2017

@johnspurlock So castv2-client eliminates the need for chromecast api emulation in electron entirely? What kind of success have you had with this?

I'm having a similar problem, except my electron app throws an error when attempting to debug with a remote console. So for example, if I run my app with --remote-debugging-port=8315, and then open localhost:8315 the app throws...

Uncaught Exception:
TypeError: Cannot read property 'substring' of undefined
    at _class._normalizeService (../node_modules/node-mdns-easy/dist/browser.js:96:57)
    at _class.serviceUp (../node_modules/node-mdns-easy/dist/browser.js:74:35)
    at emitOne (events.js:96:13)
    at module.exports.emit (events.js:188:7)
    at module.exports.internal.onMessage ../node_modules/mdns-js/lib/browser.js:108:12)
    at emitThree (events.js:116:13)
    at module.exports.emit (events.js:194:7)
    at Socket.<anonymous> (../node_modules/mdns-js/lib/networking.js:144:10)
    at emitTwo (events.js:106:13)
    at Socket.emit (events.js:191:7)

This is inconvenient since node-mdns-easy prevents me from reliably debugging 😢 (remotely at least, my use case doesn't allow me to load dev tools inside electron)

@johnspurlock
Copy link
Author

@dskvr I just needed a cast client for my electron app (not direct chrome.cast emulation), so castv2-client instead of electron-chromecast was enough for me.

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

No branches or pull requests

4 participants