67P / hyperchannel

Kosmos Chat for the Web
Mozilla Public License 2.0
20 stars 3 forks source link

Fix channel creation from updateChannelTopic method #113

Closed galfert closed 7 years ago

galfert commented 7 years ago

Closes #84

When getting a channel update message from Sockethub for a channel that didn't exist in Hyperchannel yet (happens when using the app in two tabs or two different browsers), it was creating the channel with the full id instead of just the name.

raucao commented 7 years ago

Could it be that this fixes #84 too?

galfert commented 7 years ago

Oh, yes. That's exactly it. Didn't know there was an issue for it. Updated the PR description to reference/close the issue when merged.

raucao commented 7 years ago

I tried this with joining a channel from a different tab (#84), but I get the following error in the console:

screenshot from 2017-05-08 17-37-00

galfert commented 7 years ago

That's because Sockethub changed to use context instead of platform and Hyperchannel master already includes a commit (6472bbc) to work with that. So you need to update your Sockethub instance. But then there were other problems with changed message formats. For that I just pushed the necessary adaptions. This should all work with Sockethub version 1.1.1 now.

raucao commented 7 years ago

So we should probably make Sockethub a versioned dev dependency then. I didn't see any warning about this, and Hyperchannel should probably at least tell us that the Sockethub server is too old to work with it. What do you think?

raucao commented 7 years ago

I tried with Sockethub 1.1.4 and now that throws the following in SH and doesn't even connect:

sockethub:worker:hHc7YlRYQsdtiHiBAAAC:irc caught error:  AssertionError: session.client.get first param must be a unique identifier string (job.actor.id?)
    at Object.get (/home/basti/src/sockethub/sockethub/lib/session.js:143:7)
    at IRC.join (/home/basti/src/sockethub/sockethub/node_modules/sockethub-platform-irc/index.js:530:34)
    at Domain.<anonymous> (/home/basti/src/sockethub/sockethub/lib/worker.js:111:49)
    at Domain.run (domain.js:221:14)
    at /home/basti/src/sockethub/sockethub/lib/worker.js:105:15
    at Worker.<anonymous> (/home/basti/src/sockethub/sockethub/node_modules/kue/lib/queue/worker.js:227:7)
    at Job.<anonymous> (/home/basti/src/sockethub/sockethub/node_modules/kue/lib/queue/job.js:699:12)
    at multi_callback (/home/basti/src/sockethub/sockethub/node_modules/redis/lib/multi.js:89:14)
    at Command.<anonymous> (/home/basti/src/sockethub/sockethub/node_modules/redis/lib/multi.js:116:9)
    at bound (domain.js:280:14)
    at Command.runBound [as callback] (domain.js:293:12)
    at normal_reply (/home/basti/src/sockethub/sockethub/node_modules/redis/index.js:714:21)
    at RedisClient.return_reply (/home/basti/src/sockethub/sockethub/node_modules/redis/index.js:816:9)
    at JavascriptRedisParser.returnReply (/home/basti/src/sockethub/sockethub/node_modules/redis/index.js:188:18)
    at JavascriptRedisParser.execute (/home/basti/src/sockethub/sockethub/node_modules/redis-parser/lib/parser.js:491:12)
    at Socket.<anonymous> (/home/basti/src/sockethub/sockethub/node_modules/redis/index.js:267:27) +0ms
silverbucket commented 7 years ago

this is fixed in sockethub 1.2.1 , however it's kind of messy to just put any failures in whichever thread you are testing at the moment. it would be easy for me to miss if I was not actively working on sockethub at the moment.

silverbucket commented 7 years ago

As for context vs platform, that was something that was changed years ago. There was one occurrence of it which @skddc and I ran into the other day, but it was unrelated to this issue, so I'm not sure how Hyperchannel was still using platform all this time? (perhaps it was being corrected in acitivity-streams old property renaming process).

silverbucket commented 7 years ago

@skddc the above error you mentioned was what @galfert reported earlier today https://github.com/sockethub/sockethub/issues/212#issuecomment-300117912