Skip to content
This repository has been archived by the owner on Jul 17, 2020. It is now read-only.

inital boilerplate for sharing sockets to make this effecient #249

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

inital boilerplate for sharing sockets to make this effecient #249

wants to merge 3 commits into from

Conversation

awalgarg
Copy link
Contributor

couple of things you will need to do after applying that patch: edit the loader.user.js script to point to whatever locally hosted master.js file's url is (can't be file:// obviously). need to install the new socket-saver.user.js file as well (and make sure it runs-at document-head). I will merge the two in a single script if you think it is working fine.

@Zirak
Copy link
Owner

Zirak commented Sep 21, 2015

I don't get it.

What is this?

Why is this?

What does it do?

Why did you add another userscript?

@rlemon
Copy link
Collaborator

rlemon commented Sep 21, 2015

this supposedly corrects the issue where cap is getting constantly logged out. something about sharing the socket and not creating a new one.

@awalgarg
Copy link
Contributor Author

yeah same what lemon said. chat creates a socket, then bot creates another one, so there are two sockets when there should be just one. add to it any other sockets created if cap ever opens another tab (which surely is closed instantly but still, it is made).

chat hangs due to this at the loading... page. if we save the socket (which the added userscript does), we can share it for the bot, and chat doesn't hang anymore.

@rlemon I dunno about the "keep getting logged out" thing. This only fixes freezing at the loading... page.

@awalgarg
Copy link
Contributor Author

@Zirak
Copy link
Owner

Zirak commented Sep 22, 2015

Interesting. Were you able to figure out why this happens? Is it a new thing? I create another socket sometimes myself in the chat and it never mysteriously hung up on me. Maybe it's because two sockets were created close in time to one another?

The PR generally looks good, there're two issues keeping me from merging, I'll comment inline.

@awalgarg
Copy link
Contributor Author

It happens because chat's backend is silly and randomly 404s on 100% valid requests. I'd blame it on badly implemented rate-limiting.

socket.onmessage = this.ondata.bind( this );
socket.onclose = this.socketFail.bind( this );
var socket;
// socket-saver extension specific code:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openSocket is called (eventually) from /join to, well, join a room. That's why there's the discard option...

So we either fix up /join to do something else or change openSocket or its caller to match the new behaviour.

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

Successfully merging this pull request may close these issues.

3 participants