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

Echoing messages back to the client change how embedded channels in them work. #13

Closed
wants to merge 2 commits into from

Conversation

AdrianRossouw
Copy link
Contributor

I ran into this when I was working on something in Aetherboard,
so I put it together in a simplified test case to help debug/fix the issue.

The first new test, ensures that i can embed a writeable into a message,
and then while piping the channel it is sent on,
I should always be able to pipe the writeable into something.
This is with any of the sessions, or even with just 1 graft instance.

The second test, is where I pass a return channel to the server and expect
it to echo the messages back to me.

I expect to be able to pipe the return channel messages in the same way as the
messages are piped on the remote side, or as they are piped directly from the client.

What happens is that the channel attached to the messages that gets echoed back,
can no longer be piped from.

Echoing messages back is actually one of the more common network behaviours,
such as for instance in group chat. When I post a message, I need to wait for it to be
sent back to me to be added into the single stream of conversation.

any hints where to start looking to fix this @mcollina ?

@mcollina
Copy link
Contributor

mcollina commented Oct 5, 2014

The issue is on jschan like you said in GraftJS/jschan#28, which is good and bad at the same time.
It's good, because we can make it work smoothly over here with a version bump :). It's bad because it's really complex and there are lots of variables to take into account.

I'll comment the tests here, and then move to the jsChan issue.

@@ -141,7 +140,7 @@ module.exports = function allTransportTests(buildServer, buildClient) {
client.write({ hello: 'world' });
});

it('should support sending a Readable to the other side', function(done) {
it.skip('should support sending a Readable to the other side', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you skipping this?

@mcollina
Copy link
Contributor

mcollina commented Oct 5, 2014

However, I think that want you want to achieve and the actual bug in jschan is hightlighted by https://github.com/GraftJS/graft/blob/test/echo-refactor-2/test/all_transports.js#L192-L229, which is the same of 'test/echo-refactor', but by echoing the original message.

This does not work because jschan does not support echoing a Channel, i.e. you cannot create an infinite message loop. Is it sound?

@AdrianRossouw
Copy link
Contributor Author

i ended up fixing it this way : GraftJS/aetherboard@089836d

@AdrianRossouw AdrianRossouw deleted the test/echo branch November 28, 2014 15:21
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

Successfully merging this pull request may close these issues.

2 participants