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

Allow timeout to be specified over the outgoing http connection #13

Merged
merged 1 commit into from
May 11, 2016

Conversation

HughePaul
Copy link
Contributor

Specified with timeout as an option to the model constructor or timeout as an option to a http action (save, fetch, delete) but this may get filtered out by url().
Returns an ETIMEDOUT error through the callbacks on timeout.

@@ -36,7 +36,7 @@ _.extend(Model.prototype, {

reqConf.headers = _.extend({
'Content-Type': 'application/json',
'Content-Length': data.length
'Content-Length': Buffer.byteLength(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this related to the timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah No, this should be in a separate pull request

@HughePaul HughePaul force-pushed the feature/connection-timeout branch from c0768c4 to e8e9f8b Compare May 5, 2016 16:32
…. returns an ETIMEDOUT error through the callbacks
@HughePaul HughePaul force-pushed the feature/connection-timeout branch from e8e9f8b to b65f8df Compare May 5, 2016 17:02
@gavboulton
Copy link
Contributor

This looks good. @JoeChapman @easternbloc @daniel-ac-martin what do you think?

@gavboulton
Copy link
Contributor

@joefitter
Copy link

LGTM - sorry I didn't see the @ mention before!

@easternbloc
Copy link

LGTM

@gavboulton gavboulton merged commit 694528e into master May 11, 2016
@gavboulton gavboulton deleted the feature/connection-timeout branch May 11, 2016 14:24
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

4 participants