haxetink / tink_tcp

TCP everywhere!
MIT License
13 stars 6 forks source link

[neko] No response from the connection source #3

Closed kevinresol closed 8 years ago

kevinresol commented 8 years ago

The following code works on nodejs. But on neko there is no response.

package;

import haxe.io.*;
import tink.io.*;
import tink.io.StreamParser;
import tink.tcp.*;

using StringTools;
using tink.CoreApi;

class Main
{
    public static function main()
    {
        Connection.tryEstablish({host:'www.google.com', port:80}, Worker.get(), Worker.get()).handle(function(o) switch o {
            case Success(cnx):
                cnx.source.parse(new Parser()).handle(function(o) switch o {
                    case Success(d): 
                        trace(d.data);
                    case Failure(f): 
                        trace(f);
                });

                ("GET /\r\n":Source).pipeTo(cnx.sink).handle(function(o) switch o {
                    case SinkFailed(e) | SourceFailed(e):
                        trace(e);
                    case SinkEnded:
                        trace(new Error('sink ended'));
                    case AllWritten:
                });
            case Failure(f):
                trace(f);
        });

    }
}

class Parser extends ByteWiseParser<String> {
    var out:BytesOutput;
    public function new() {
        out = new BytesOutput();
        super();
    }
    override function read(c:Int) {
        if(c == -1) return Done(out.getBytes().toString());
        out.writeByte(c);
        return Progressed;
    }
}
back2dos commented 8 years ago

That was a bug in tink_io. Punishment for me trying to be smart. It should work now.

If you spot bugs like this, I would be extremely grateful if you could go the extra mile and push a failing tests (on a separate branch if you wish, but if the lib is broken, then I don't mind the build reflecting that). The coverage of most tink libs is practically zero, so this would help a lot ;)

kevinresol commented 8 years ago

Sure I can add tests. Haxe's build-in unit test doesn't handle async tasks though. I prefer buddy. Do you have preference?

back2dos commented 8 years ago

To be honest, I think all Haxe testing frameworks somehow suck at one point or another, but all of them are good enough to get the job done. As far as async goes, buddy looks like the best solution. Personally, I'm really not very fond of the Should notation. But please feel free to use whatever you want. While the best code is no code at all, the opposite is true for tests. The more, the better. I will be more than happy to have the problem of our tests becoming too complex :D

kevinresol commented 8 years ago

I have added some tests. But they are failing.

On nodejs, it is fine. On neko, the following behaviour is observed:

Test Code In Buddy Suite -lib tink_runloop Result
Yes No it can connect but no response
Yes Yes it can't even connect
(the worker here doesn't work)
No No it can connect but no response
No Yes success

I am not sure why the buddy suite even matters...

back2dos commented 8 years ago

Buddy and tink_runloop seem to be clashing. That's too bad. I'll see what I can do. Thanks a lot for the tests :)

kevinresol commented 8 years ago

Maybe @ciscoheat can give us some hints I think I read somewhere that buddy is using threads behind the scene.

ciscoheat commented 8 years ago

I'm happy to help. Maybe you can try to set this.timeoutMs = 0 in the tests. That disables threading, since Neko must create an extra thread to handle async timeouts.

And since you mention a run loop: I'm using promhx for running the tests, it has some special things going on, having an event loop for example. Maybe some conflict there?

(If you want to avoid the should notation, you can use utest.Assert instead, it works straight out-of-the-box.)

Where are the tests? Got a branch that I can check out?

kevinresol commented 8 years ago

@ciscoheat We are talking about the test here https://github.com/haxetink/tink_tcp/blob/master/tests/TestIssue3.hx

ciscoheat commented 8 years ago

Ok thanks, but that's the node test, right? Since Neko doesn't have haxe.Timer.delay?

kevinresol commented 8 years ago

Oh i was testing neko. I am using haxe dev, is that the reason timer.delay is there?

Anyway that piece of code is irrelevant, i was just using that to check if the test code is exited early without waiting for a reaponse

On Thu, 12 May, 2016 18:03 Andreas notifications@github.com wrote:

Ok thanks, but that's the node test, right? Since Neko doesn't have haxe.Timer.delay?

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub https://github.com/haxetink/tink_tcp/issues/3#issuecomment-218713764

back2dos commented 8 years ago

Well, there's many things clashing here.

  1. the latest haxe dev indeed has haxe.Timer on neko, because it ships with its own run loop and timer implementation on top of it. I have no idea whether or not those two might be clashing.
  2. somehow the presence of buddy seems to make tink_runloop hang, but I will see about that.
  3. the tests hang without tink_runloop because you first initiate a read from the socket and then write the http request. Without the runloop, the execution hangs on the read (because with eager workers, so the pipe loops until the end is reached), so the write is never issued. I've reversed order and it works now.

It seems that the presence of buddy makes tink_runloop enter some hanging state. I think that's because both libraries fumble with the main entry point and obviously it doesn't end well for tink_runloop :D

ciscoheat commented 8 years ago

Sorry about that fumbling. :) I've made an example for running the tests manually, but for multi-platform/target tests, that could be a hassle... https://github.com/ciscoheat/buddy#can-i-run-the-tests-manually-without-generating-main

back2dos commented 8 years ago

@ciscoheat Thanks for the hint. I've got it to run locally now. Just have to get Travis to like me again.

As for the clash of the libs, I've opened an issue here for those interested: https://github.com/haxetink/tink_runloop/issues/1

back2dos commented 8 years ago

Ok, it works now \o/

Thanks a lot for the help guys!

ciscoheat commented 8 years ago

No problem :) Just to be sure, is it working with Travis, even though it's not returning an exit code? The generated main is using Sys.exit: https://github.com/ciscoheat/buddy/blob/master/src/buddy/internal/GenerateMain.hx#L220 (maybe I should add that to the example)

back2dos commented 8 years ago

Oh, good catch! I was happy enough to see 0 failures but you're right, I need to exit correctly of course.

back2dos commented 8 years ago

Ok, seems to work.