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

Various issues #5

Closed
ccoenraets opened this issue May 24, 2017 · 12 comments
Closed

Various issues #5

ccoenraets opened this issue May 24, 2017 · 12 comments

Comments

@ccoenraets
Copy link

  • readme says to use require('cometd-nodejs-client').adapt(), but the module doesn't have an adapt() function.
  • I just used require('cometd-nodejs-client'), but then ran into an 'XMLHttpRequest is not defined' error
  • Patched that error with global.XMLHttpRequest = window.XMLHttpRequest; in my module, but then ran in 'https not supported' error
  • Replaced var http = require('http'); with var http = require('https'); in cometd-nodejs-client.js but now running in 'Length Required' error.

Am I using this right or is there another way to use this module?
Thanks

@sbordet
Copy link
Member

sbordet commented May 24, 2017

The module does have an adapt() function (https://github.com/cometd/cometd-nodejs-client/blob/master/cometd-nodejs-client.js#L2) and you must call it to have the module work properly.

@ccoenraets
Copy link
Author

The version installed via npm (1.0.0-BETA0) does not have an adapt() function. Will try with the version from the repo. Thanks.

@sbordet
Copy link
Member

sbordet commented May 24, 2017

Install latest version on tag beta.

@ccoenraets
Copy link
Author

OK, got 1.0.0-BETA3 with adapt() function. I'm still getting ReferenceError: XMLHttpRequest is not defined. I'm patching it with global.XMLHttpRequest = window.XMLHttpRequest; in my module, but then getting Error: Protocol "https:" not supported. Expected "http:". Thanks.

@sbordet
Copy link
Member

sbordet commented May 24, 2017

Install cometd at tag beta as well. Sorry, stuff is still experimental :)

@ccoenraets
Copy link
Author

No problem. Appreciate the hard work on the library. The beta of cometd fixed the first problem (XMLHttpRequest is not defined), but it looks like the adapter does not support https. Still getting Protocol "https:" not supported. Expected "http:".

@ccoenraets
Copy link
Author

Quick fix to make it work in my specific use case. require('https'), and add Content-Length header: _config.headers['Content-Length'] = JSON.stringify(data).length;.

@sbordet
Copy link
Member

sbordet commented May 25, 2017

Can you please detail how you get the "https:" not supported. Expected "http:" error ?

@sbordet
Copy link
Member

sbordet commented May 25, 2017

I think I understand the https error now... I guess you're using https: URLs, but in the code I only use the http module to send requests, while I should switch using either the http module or the https module depending on the URL scheme.

The suggestion you made about the Content-Length header is not correct since the length of the string is not equal to the length in bytes in case of non-ASCII characters.

@sbordet
Copy link
Member

sbordet commented May 25, 2017

@ccoenraets I filed #6 to isolate the https issue.
Can you please try the latest code (from HEAD, copy and paste the new code - it's not published to NPM yet) and report if it works for you ?

@glennschler
Copy link

glennschler commented May 25, 2017

A possible suggestion for this https issue, is option to pass in an Agent object. Then no effort to decide inside cometd-nodes-client. Similar to the way Request module options work. The caller decides the many possible configurations of the http agent. Plus then will be able to instantiate many cometd clients per process with unique sandboxed agent objects. Also important, just like Request, allow optional Cookie store to be passed in.

I can separate my comments into Agent for issue #6, and another for Cookies.

Request modules optional options:

  • jar - define your custom cookie jar
  • agent - http(s).Agent instance to use




@sbordet
Copy link
Member

sbordet commented May 25, 2017

@glennschler, the goal of this library is not to be a full rewrite of the browser CometD client, but just perform the necessary adaptations to make it work in a NodeJS environment. It basically has to just "fake" XMLHttpRequest.

From fixing #6, passing an Agent is not enough, since you have to switch on the http and https modules as well.

Cookies have been solved in #4.

@sbordet sbordet closed this as completed Jul 4, 2017
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

3 participants