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

no event on connection loss #27

Closed
antonymayi opened this issue Jan 2, 2014 · 22 comments
Closed

no event on connection loss #27

antonymayi opened this issue Jan 2, 2014 · 22 comments

Comments

@antonymayi
Copy link

Hi,

The fail() handler is supposed to be called on connection loss but it doesn't seem so. I get it called ok for example when the connection couldn't establish at all but not when it is terminated in the middle of a stream.

For example I have a service the receives JSON array of IDs (using POST) and returns streamed JSON array of requested objects:

oboe({
          method: "POST",
          url: "http://my_url",
          body: [id1, id2, id3, id4, id5]
        })
          .node('!.*', function (my_obj) {
             console.obj('obj ' + my_obj['id'] + 'returned');
           })
          .fail(function (error) {
            console.log('error');
          });

If I kill the service for example after receiving second object the fail() is not called and I have no way to handle that situation. This works ok if I run it when the service is already dead - the fail() handler gets called in this case.

Thanks, Antony.

@jimhigson
Copy link
Owner

Looks like something to take a look at. Are you running in Node.js and/or a browser? If a browser, do you get the same behaviour on different browsers?

@antonymayi
Copy link
Author

Running in browser, tried Chrome and Firefox, getting same from both.

@jimhigson
Copy link
Owner

Ok, thanks for the report. Going to look at this ASAP, probably tomorrow.

Keeping the data fetched so far when the network fails is one of the major benefits of streaming. If we don't know when it goes down it makes impossible to request the bits that were missed.

Pre-launch website example:
http://oboejs.jit.su/why/#dropped-connections

@jimhigson
Copy link
Owner

Couple more questions. I'm trying to reproduce this in an integration test:

What kind of server are you connecting to and how are you killing it?

Do you get a fail event if you are in, say, jQuery and kill the server process?

Cheers,
Jim

@jimhigson
Copy link
Owner

Ok, I think jQuery will fail properly because of this:

https://github.com/jquery/jquery/blob/master/src/ajax/xhr.js#L110
http://www.w3.org/TR/XMLHttpRequest/#event-xhr-error

Easy bug to fix but not so easy to write an integration test for. Need to work out how to fake a network failure from a Node http server.

@jimhigson
Copy link
Owner

Could you confirm if the xhrerror branch fixes the problem you're seeing? If this fixes it I'll merge into master and tag a new minor release.

@antonymayi
Copy link
Author

I am running it against django test server (Python WSGIServer) which I just kill using Ctrl-C in the middle of the response (the response is being generated slowly object by object so I have enough time to kill it).

Unfortunately I still don't get the fail handler called upon the stream termination even with this branch.

@antonymayi
Copy link
Author

And yes, when trying ajax instead of oboe the fail() (or rather error() in case of ajax) handler gets called when the server is killed while generating the response.

@jimhigson
Copy link
Owner

I've run quite a few experiments here and I'm seeing the .fail() handler called if I kill a Node.js server while a request is ongoing, for either master or the branch. I can't get it not to fail even if I hit it from another computer and bring the wifi down. Only idea I have left is that django/Python is doing something different from Node to close the connection when it traps the SIGINT.

Are you able to provide a stripped-down example of the failure? For example, with a server that gives a streaming JSON resource and a tiny webpage that uses jQuery and Oboe to fetch it. This would help a lot in diagnosing the problem.

Alternatively, if you could get a failure in the unminified jQuery 2.0.3 and post a screenshot of a stack trace from inside the error handler so I can see where jQ is getting the failure from the underlying XHR, that'd help a lot too. From the code it looks like Oboe is catching all the failures that jQ is, but you must have found one that is slipping through somehow.

Thanks for your help so far,
Jim

@antonymayi
Copy link
Author

Ok, I was trying to write a mini demo to show the issue using primitive http server based on python's BaseHTTPServer (not familiar with Node.js 👎 ) and minimal oboe code. I was pulling my hairs for couple of hours because I was not able to reproduce this issue of my main app - the fail() handler was correctly called upon termination of the server - until I realized the issue only occurs when the stream is interrupted exactly between JSON nodes, that oboe 'listens' to. If it is terminated while part of a node has been transmitted, the fail() gets called ok.

Demo code

Client

<html>
<body>
  <script src="jquery.min.js"></script>
  <script src="oboe-browser.min.js"></script>
  <script>
    $( document ).ready(function() {
      oboe({
        url: "http://localhost:8000",
      })
        .node('!.*', function (data) {
          console.log('received: ' + data);
        })
        .done(function (data) {
          console.log('done: ' + data);
        })
        .fail(function (error) {
           console.log('fail: ' + error.thrown);
        });
    });
  </script>
</body>
</html>

Server

Tested on linux, not sure how os.kill() responds on other platforms. It simulates sending of JSON [[1,2,3],[4,5,6]] where the stream is terminated either:

  • after the first sub-array [1,2,3] (first os.kill()) - this is when oboe misses the fail()
  • or (if the first os.kill() is commented out) in the middle of second sub-array - now oboe gets the fail() called ok
#!/usr/bin/python
import BaseHTTPServer
import time
import os
import signal

class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
    def do_OPTIONS(self):
        self.send_response(200)
        self.send_header('Access-Control-Allow-Origin', '*')
        self.send_header('Access-Control-Allow-Headers', 'X-Requested-With')
        self.end_headers()

    def do_GET(self):
        self.send_response(200)
        self.send_header('Content-type', 'application/json')
        self.send_header('Access-Control-Allow-Origin', '*')
        self.end_headers()
        self.wfile.write('[\n')
        self.wfile.write('[1,2,3],\n')
        time.sleep(2)
        # following line simulates termination between nodes (oboe's fail() miss)
        # comment it out to simulate termination in middle of a node (fail() ok)
        os.kill(os.getpid(), signal.SIGKILL)
        self.wfile.write('[4,5')
        os.kill(os.getpid(), signal.SIGKILL)
        self.wfile.write(',6]\n]')

httpd = BaseHTTPServer.HTTPServer(
        ('localhost', 8000),
        RequestHandler)

while True:
    httpd.handle_request()

Results

In first case (stream killed between nodes using the first os.kill()) the console displays just:

received: 1,2,3 

no error output from fail() handler.

In second case (first os.kill() commented out in my python testserver) the console displays:

received: 1,2,3
fail: Error: Unexpected end
Line: 3
Column: 5
Char: 

so fail() gets called this time.

@jimhigson
Copy link
Owner

This will be extremely useful, thank you.

I'm on OSX which I'd guess is similar enough to Linux to reproduce the problem. If not I have a Linux box as well to run the server from. Going to take a look see what is happening inside Oboe.

@jimhigson
Copy link
Owner

Still having problems reproducing the problem. I've found a related bug (but only in my branch, not master) where the .fail callback is called twice. Determined to squash this one though.

Unfortunately I know almost no Python. How do I get the same server as is serving the streamed JSON to serve the HTML page so as to be able to hit it under the same-origin policy? At the moment I'm fronting your Python service with Apache mod_rewrite for the static HTML and I wonder if that is influencing the results.

@antonymayi
Copy link
Author

just load the html from disk using file://path/to/the/html and not from any server.

if you don't run the python server on your localhost just change the listening address in the code to 0.0.0.0 and then the oboe url to whatever IP is the python running on.

@antonymayi
Copy link
Author

I mean point your browser to file://path/to/the/html

@antonymayi
Copy link
Author

re same-origin - the headers the server returns allow mixed origins for the sake of this demo...

@jimhigson
Copy link
Owner

Ok, am now able to reproduce this problem.

I can make some observations:

  1. It seems like the Python server is sending a correct message, at least in terms of HTTP. The Chrome inspector shows this as a normal, successful HTTP request.
  2. When Oboe does catch the error, it is because Clarinet (which it uses internally) is complaining that the JSON is not valid. This looks like an issue in Clarinet that it doesn't always throw error on JSON which is closed before it is complete.
  3. jQuery likewise only catches this because it thinks the HTTP is successful but then fails to parse the JSON.
  4. There's a new version of Clarinet. I'll try that. Otherwise I should be able to either patch Clarinet and make a PR or fix it in Oboe.

@jimhigson
Copy link
Owner

The good news is that because this issue is actually a JSON parsing problem (not a network issue) it is quite easy to write the automated test so that it doesn't come up again.

@jimhigson
Copy link
Owner

Caught the bug in component tests here

@jimhigson
Copy link
Owner

I've patched Clarinet now to fix this, if you want the fix quick let me know, you can build Oboe against my own version of Clarinet. I'm expecting to have a version of Oboe.js tagged without this bug perhaps next week.

@antonymayi
Copy link
Author

perfect, I can wait for the official release, thanks.

@jimhigson
Copy link
Owner

Pull request into Clarinet: dscape/clarinet#20

@jimhigson
Copy link
Owner

Pull request was merged into Clarinet and this is fixed in Oboe.js as of v1.12.2

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

No branches or pull requests

2 participants