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.

Sign up for GitHub

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

added request.setTimeout support #80

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

added request.setTimeout support #80

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c9ce6c
added request.setTimeout support
brandonrobertz Jan 25, 2015
fa18e1f
code simplifications
brandonrobertz Jan 27, 2015
626f305
tests around request setTimeout
brandonrobertz Jan 28, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,24 @@ Request.prototype.end = function (s) {
}
};

Request.prototype.setTimeout = function(msecs, callback) {
if (callback) this.once('timeout', callback);

var self = this;
function emitTimeout() {
self.emit('timeout');
}

// set and handle the timeout signals on xhr
if (this.xhr) {
if (this.timeoutCb) {
this.xhr.ontimeout = this.timeoutCb;
Copy link
Member

Choose a reason for hiding this comment

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

Could be:

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

Depends on whether you prefer anonymous closures though.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Right, sorry. My mistake. LGTM otherwise :)

}
this.timeoutCb = emitTimeout;
this.xhr.timeout = msecs;
}
};

// Taken from http://dxr.mozilla.org/mozilla/mozilla-central/content/base/src/nsXMLHttpRequest.cpp.html
Request.unsafeHeaders = [
"accept-charset",
Expand Down