Skip to content

Add timeout support to Http protocol #439

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

Merged
merged 4 commits into from
Aug 9, 2019
Merged

Add timeout support to Http protocol #439

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6cfb63
feat(http): add timeout
Aschen Aug 5, 2019
3a0a2cd
update features submodule
Aschen Aug 6, 2019
5faaa66
add timeout unit
Aschen Aug 8, 2019
41f3946
Apply suggestions from code review
Aschen Aug 9, 2019
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
1 change: 1 addition & 0 deletions doc/6/protocols/http/constructor/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Http protocol connection options.
| `port` | <pre>number</pre><br/>(`7512`) | Kuzzle server port |
| `sslConnection` | <pre>boolean</pre><br/>(`false`) | Use SSL to connect to Kuzzle server |
| `customRoutes` | <pre>object</pre><br/>(`{}`) | Add custom routes <SinceBadge version="6.2.0"/> |
| `timeout` | <pre>number</pre><br/>(`0`) | Connection timeout <SinceBadge version="6.2.1"/> |
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the unit? And does '0' mean "no timeout"?


**Note:**

Expand Down
19 changes: 10 additions & 9 deletions doc/6/protocols/http/properties/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ order: 10

# Properties

| Property name | Type | Description |
| -------------------- | -------- | ---------------------|
| `connected` | <pre>boolean</pre> | Always returns `true` |
| `host` | <pre>string</pre> | Kuzzle server host |
| `http` | <pre>object</pre> | Returns a list of available routes <DeprecatedBadge version="6.2.0"/> |
| `routes` | <pre>object</pre> | Returns a list of available routes <SinceBadge version="6.2.0"/> |
| `port` | <pre>number</pre> | Kuzzle server port |
| `protocol` | <pre>string</pre> | `https` or `http` |
| `ssl` | <pre>boolean</pre> | `true` if ssl is active |
| Property name | Type | Description | Get/Set |
| -------------------- | -------- | ---------------------| ---------|
| `connected` | <pre>boolean</pre> | Always returns `true` | Get |
| `host` | <pre>string</pre> | Kuzzle server host | Get |
| `http` | <pre>object</pre> | Returns a list of available routes <DeprecatedBadge version="6.2.0"/> | Get |
| `routes` | <pre>object</pre> | Returns a list of available routes <SinceBadge version="6.2.0"/> | Get |
| `port` | <pre>number</pre> | Kuzzle server port | Get |
| `protocol` | <pre>string</pre> | `https` or `http` | Get |
| `ssl` | <pre>boolean</pre> | `true` if ssl is active | Get |
| `timeout` | <pre>number</pre> | Connection timeout <SinceBadge version="6.2.1"/>| Get/Set |
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

2 changes: 1 addition & 1 deletion features/features
Open in desktop
Submodule features updated 1 files
+1 −1 documents.feature
15 changes: 14 additions & 1 deletion src/protocols/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class HttpWrapper extends KuzzleAbstractProtocol {

this._routes = {};

this._timeout = options.timeout || 0;

this.customRoutes = options.customRoutes || {};

for (const [controller, definition] of Object.entries(this.customRoutes)) {
Expand Down Expand Up @@ -47,6 +49,14 @@ class HttpWrapper extends KuzzleAbstractProtocol {
return true;
}

get timeout () {
return this._timeout;
}

set timeout (timeout) {
this._timeout = timeout;
}

/**
* Connect to the server
*/
Expand Down Expand Up @@ -230,7 +240,8 @@ class HttpWrapper extends KuzzleAbstractProtocol {

return httpClient.request(url, method, {
headers,
body: payload.body
body: payload.body,
timeout: this._timeout
})
.then(response => JSON.parse(response.body));
}
Expand All @@ -241,6 +252,8 @@ class HttpWrapper extends KuzzleAbstractProtocol {
xhr = new XMLHttpRequest(),
url = `${this.protocol}://${this.host}:${this.port}${path}`;

xhr.timeout = this._timeout;

xhr.onreadystatechange = () => {
if (xhr.readyState === 4 && xhr.status === 0) {
reject(new Error('Cannot connect to host. Is the host online?'));
Expand Down
58 changes: 51 additions & 7 deletions test/protocol/http.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,26 +483,59 @@ describe('HTTP networking module', () => {
protocol._sendHttpRequest('VERB', '/foo/bar');

should(httpRequestStub).be.calledOnce();
should(httpRequestStub).be.calledWith('http://address:1234/foo/bar', 'VERB', { body: undefined, headers: {'Content-Length': 0} });
should(httpRequestStub).be.calledWith(
'http://address:1234/foo/bar',
'VERB',
{
body: undefined,
headers: { 'Content-Length': 0 },
timeout: 0
});
});

it('should call http.request with a body', () => {
const body = 'http request body';
protocol._sendHttpRequest('VERB', '/foo/bar', {body});

should(httpRequestStub).be.calledOnce();
should(httpRequestStub).be.calledWith('http://address:1234/foo/bar', 'VERB', {body: 'http request body', headers: {'Content-Length': body.length}});
should(httpRequestStub).be.calledWith(
'http://address:1234/foo/bar',
'VERB',
{
body: 'http request body',
headers: { 'Content-Length': body.length },
timeout: 0
});
});

it('should call http.request with configured timeout', () => {
protocol.timeout = 42000;
protocol._sendHttpRequest('VERB', '/foo/bar');

should(httpRequestStub).be.calledOnce();
should(httpRequestStub).be.calledWith(
'http://address:1234/foo/bar',
'VERB',
{
body: undefined,
headers: { 'Content-Length': 0 },
timeout: 42000
});
});

it('should call http.request with a body and some headers', () => {
const body = 'http request body';
protocol._sendHttpRequest('VERB', '/foo/bar', {body, headers: {foo: 'bar'}});

should(httpRequestStub).be.calledOnce();
should(httpRequestStub).be.calledWith('http://address:1234/foo/bar', 'VERB', {
body: 'http request body',
headers: {'Content-Length': body.length, foo: 'bar'}
});
should(httpRequestStub).be.calledWith(
'http://address:1234/foo/bar',
'VERB',
{
body: 'http request body',
headers: { 'Content-Length': body.length, foo: 'bar' },
timeout: 0
});
});

it('should reject the request in case of error', () => {
Expand Down Expand Up @@ -537,7 +570,8 @@ describe('HTTP networking module', () => {
open: sinon.stub(),
send: sinon.stub(),
setRequestHeader: sinon.stub(),
onreadystatechange: sinon.stub()
onreadystatechange: sinon.stub(),
timeout: 0
};
// eslint-disable-next-line no-native-reassign, no-global-assign
XMLHttpRequest = function() {
Expand Down Expand Up @@ -566,6 +600,16 @@ describe('HTTP networking module', () => {
should(xhrStub.setRequestHeader).not.be.called();
});

it('should call XMLHttpRequest with configured timeout', () => {
protocol.timeout = 42000;

protocol._sendHttpRequest('VERB', '/foo/bar');

should(xhrStub.open).be.calledOnce();
should(xhrStub.timeout).be.eql(42000);
});


it('should call XMLHttpRequest with a body', () => {
const body = 'http request body';
protocol._sendHttpRequest('VERB', '/foo/bar', {body});
Expand Down