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

timeout event, and timeout option implemented. added docs & example #130

Merged
merged 2 commits into from Dec 11, 2013

Conversation

bosky101
Copy link
Contributor

@bosky101 bosky101 commented Sep 4, 2013

I was serially testing GET's on a million websites, and gradually came across my test app stalling with the same pattern.

It turned out that restler has/had no option for timeout. This is a show-stopper for large/production scenarios.

eg: The following code, stalls forever, without throwing an error or

  rest.get('http://tianji.com').on('complete', function(){ 
    console.log('never entered here');
  });

infact it did not emit any known events either. namely error or abort.

 rest.get('http://tianji.com').on('error' function(ms){
   sys.puts('neither found an error');
  }).on('abort' function(ms){
   sys.puts('nor an abort');
  }).on('complete',function(data,response){
   sys.puts('never reached here either');
 });

I also saw previous mentions requesting timeout feature for restler.
eg: #3 one of the oldest open bugs/issues.

So i went ahead, forked and fixed this.

My commit includes :

  • added the 'timeout' event
  • added the {timeout:Ms} option
  • added documentation for timeout
  • added an example for timeout

Here's an example.

 rest.get('http://tianji.com',{timeout: 10000}).on('timeout' function(ms){
   sys.puts('did not return within '+ms+' ms');
 }).on('complete',function(data,response){
   sys.puts('life goes on');
 });

Will love to see this reviewed for a possible merge.

~Bosky @bhaskerkode

@ohgyun
Copy link

ohgyun commented Sep 24, 2013

+1

2 similar comments
@pyrat
Copy link

pyrat commented Sep 27, 2013

+1

@darthdeus
Copy link

+1

@easternbloc
Copy link
Collaborator

I'm happy to merge this with some tests. Any chance you could knock some up?

easternbloc added a commit that referenced this pull request Dec 11, 2013
timeout event, and timeout option implemented. added docs & example
@easternbloc easternbloc merged commit 0ca56f7 into danwrong:master Dec 11, 2013
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.

None yet

5 participants