added request.setTimeout support

[brandonrobertz]

This is a port of node's request http setTimeout method.

[dcousens]

Could be:

this.xhr.ontimeout = this.emit.bind(this, 'timeout')

Depends on whether you prefer anonymous closures though.

[brandonrobertz]

I pretty much ported directly from the node implementation, but used the xhr-provided timeout scaffolding. The this.once('timeout', callback) just adds a one-time event handler. It shouldn't call back immediately.

I'm gonna be doing a little more testing with your binding/emit code. It's a lot cleaner.

[dcousens]

Right, sorry. My mistake. LGTM otherwise :)

[brandonrobertz]

I've tested fa18e1f with my target libs. Looking good.

[dcousens]

Could you add a basic test for this?
On 27 Jan 2015 2:45 pm, "B Roberts" [email protected] wrote:

I've tested fa18e1f
fa18e1f
with my target libs.

—
Reply to this email directly or view it on GitHub
#80 (comment)
.

[brandonrobertz]

Yeah, I'll work on something. It's complicated, because so much of this depends on XMLHttpRequest. But it's worth it, so I'll see what I can put together. The existing tests mock out window with stubs, so I think I'll have to take a similar route with xhr.

[brandonrobertz]

I wrote two tests. One to make sure request.setTimeout sets the correct xhr properties and another that simulates xhr timeout activity during send.

[brandonrobertz]

If nobody objects or has comments ... @substack ?

[vvo]

@substack any comments on this? thx

[connor4312]

Would it be possible to get this merged?

[vvo]

I believe someone willing to do it would have to ask @substack on twitter for repository and maintainer rights on this project. If he feels like he will have the bandwidth and will to do so.

[brandonrobertz]

I spoke with substack on IRC a while back about this and it turns out browserify is now using stream-http when you require('http'). So it looks like this codebase is deprecated?

[vvo]

So it looks like this codebase is deprecated?

Yes if you are using the latest browserify. Some people may not but not sure we need to fix this :)

Then the next step would be to deprecate this module officially in the README, close issues and PRs.