-
Notifications
You must be signed in to change notification settings - Fork 252
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
update node-xmpp-client #261
Conversation
sonnyp
commented
Oct 28, 2015
- node-xmpp-core is not needed (and possibly harmful as you possibly could be using a different version as node-xmpp-client)
- node-xmpp-client now installs on Node.js 4
@@ -24,8 +24,6 @@ | |||
fs = require "fs" | |||
{bind, isString, isRegExp} = require "underscore" | |||
xmpp = require 'node-xmpp-client' | |||
xmpp.Element = xmpp.ltx.Element | |||
xmpp.JID = require('node-xmpp-core').JID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attempted to use this PR on my version of Hubot running on node v4.2.2. With these two lines removed, I fail at startup with the error below. If I add these lines back, then it will start successfully. The node-xmpp issue that was preventing install on v4 is fixed with this patch.
Thanks for the fix.
[Wed Nov 04 2015 09:56:06 GMT-0800 (PST)] ERROR TypeError: xmpp.Element is not a function
at Connector.module.exports.Connector.setAvailability (./node_modules/hubot-hipchat/src/connector.coffee:171:18, <js>:153:16)
at Connector.onOnline (./node_modules/hubot-hipchat/src/connector.coffee:392:3, <js>:341:10)
at emitOne (events.js:77:13)
at Client.emit (events.js:169:7)
at Client.useFeatures (./node_modules/node-xmpp-client/lib/Client.js:345:14)
at Client._handleSessionState (./node_modules/node-xmpp-client/lib/Client.js:234:14)
at Client.onStanza (./node_modules/node-xmpp-client/lib/Client.js:217:14)
at emitOne (events.js:77:13)
at Connection.emit (events.js:169:7)
at Connection.onStanza (./node_modules/node-xmpp-core/lib/Connection.js:370:14)
at StreamParser.<anonymous> (./node_modules/node-xmpp-core/lib/Connection.js:226:14)
at emitOne (events.js:77:13)
at StreamParser.emit (events.js:169:7)
at SaxLtx.<anonymous> (./node_modules/node-xmpp-core/lib/StreamParser.js:59:22)
at emitOne (events.js:77:13)
at SaxLtx.emit (events.js:169:7)
at SaxLtx._handleTagOpening (./node_modules/ltx/lib/parsers/ltx.js:32:14)
at SaxLtx.write (./node_modules/ltx/lib/parsers/ltx.js:95:18)
at StreamParser.write (./node_modules/node-xmpp-core/lib/StreamParser.js:128:21)
at Connection.onData (./node_modules/node-xmpp-core/lib/Connection.js:306:21)
at emitOne (events.js:82:20)
at TLSSocket.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:146:16)
at TLSSocket.Readable.push (_stream_readable.js:110:10)
at TLSWrap.onread (net.js:523:20)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you updated node-xmpp-client?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did an rm -rf node_modules
followed by an npm install
with the dependency in package.json set to "hubot-hipchat": "git+https://github.com/sonnyp/hubot-hipchat.git#update-node-xmpp"
. Still seeing the above error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I think I know what it is, will make a new node-xmpp-client release later in a sec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdouglass I released node-xmpp-client 2.1.0 and bump the version in this PR; should be fixed; can you try again?
thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 that's working for me. thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for testing!
@rbergman please consider testing and merging this. |
I can confirm these fixes work. |
@rbergman are you handling updates to this repo now or is there someone else who should be merging? |
I poked them on twitter https://twitter.com/sonnypiers/status/667291934029803521 |
Creates a local branch holding the fixes outlined in hipchat#261 until upstream merges.
Sorry, maintaining this is a non-priority for Atlassian at this time, and I've been lacking the downtime to keep up with it on the side as a personal project. I'll try to keep merging PRs when the community steps up like this as I can though. Sorry for the delayed response -- I don't keep a close eye on this project like I used to, given that Atlassian has been focusing on much deeper integration solutions in the last year via the HipChat Connect platform. |
@rbergman Thanks for the continued support. Can you guys appoint at least 1 active collaborate to help triage and merge PRs ? |
I've thought about that, but there have been virtually no recurring contributors by the looks of our merge history. If someone wants to start actively maintaining this, probably the best bet would be to create an active fork and announce it via an issue here. If it gains traction, I'd look into getting approval to point to it from this repo via the README. |