Skip to content
This repository was archived by the owner on Jul 6, 2018. It is now read-only.

No closures #23

Closed
wants to merge 3 commits into from
Closed

No closures #23

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99a32a9
more perf updates
mcollina Dec 5, 2016
16e0c23
Removed most closures.
mcollina Dec 5, 2016
839a3f2
From Map() to basic {}.
mcollina Dec 6, 2016
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
Next Next commit
more perf updates
  • Loading branch information
@mcollina
mcollina committed Dec 5, 2016
commit 99a32a9ccf9fea85a9c2e02b16ca26e07d404e12
46 changes: 25 additions & 21 deletions lib/internal/http2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1307,17 +1307,19 @@ function initializeTLSOptions(options) {
return options;
}

function onErrorSecureServerSession(err, conn) {
if (!this.emit('clientError', err, conn))
conn.destroy(err);
}

class Http2SecureServerSession extends TLSServer {
constructor(options, requestListener) {
super(initializeTLSOptions(options), connectionListener);
this[kOptions] = options;
this.timeout = kDefaultSocketTimeout;
if (typeof requestListener === 'function')
this.on('request', requestListener);
this.on('tlsClientError', (err, conn) => {
if (!this.emit('clientError', err, conn))
conn.destroy(err);
});
this.on('tlsClientError', onErrorSecureServerSession);
}

setTimeout(msecs, callback) {
Expand Down Expand Up @@ -1475,9 +1477,8 @@ class Http2ClientSession extends EventEmitter {
this[kSocket] = socket;

const session = this[kSession] = createClientSession(options, socket);
socket.once('error', (error) => {
console.log(error);
});
// TODO remove this
socket.once('error', console.log);
socket.on('resume', socketOnResume);
socket.on('pause', socketOnPause);
socket.on('drain', socketOnDrain);
Expand Down Expand Up @@ -1605,20 +1606,7 @@ class Http2ClientRequest extends Http2Outgoing {
const _handle = this.stream.session.request(mapToHeaders(this[kHeaders]), true);
if (_handle instanceof http2.Http2Stream) {
this[kId] = _handle.getId();
this.stream.once('handle', () => {
if (this[kTrailers] instanceof Map) {
for (const v of this[kTrailers]) {
const key = String(v[0]);
const value = v[1];
if (Array.isArray(value) && value.length > 0) {
for (const item of value)
this.stream.addTrailer(key, String(item));
} else {
this.stream.addTrailer(key, String(value));
}
}
}
});
this.stream.once('handle', addTrailers)
this.stream._handle = _handle;
}
}
Expand All @@ -1629,6 +1617,22 @@ class Http2ClientRequest extends Http2Outgoing {
}
}

function addTrailers () {
const request = this[kRequest];
if (request[kTrailers] instanceof Map) {
Copy link
Contributor

@evanlucas evanlucas Dec 5, 2016

Choose a reason for hiding this comment

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

should we use the internal util for this? Is there a chance that the Map was created in a different context?

Copy link
Member

@jasnell jasnell Dec 5, 2016

Choose a reason for hiding this comment

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

@evanlucas ... no, the Map here should always be created in the same context. We could likely just have this be a truthy check to be honest.

for (const v of request[kTrailers]) {
const key = String(v[0]);
const value = v[1];
if (Array.isArray(value) && value.length > 0) {
Copy link
Contributor

@evanlucas evanlucas Dec 5, 2016

Choose a reason for hiding this comment

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

Doesn't for of deopt for arrays?

Copy link
Member

@jasnell jasnell Dec 5, 2016

Choose a reason for hiding this comment

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

hmm... I believe it does... (or at least did... I don't know if it still does)

for (const item of value)
this.addTrailer(key, String(item));
} else {
this.addTrailer(key, String(value));
}
}
}
}

class Http2ClientResponse extends Http2Incoming {
constructor(stream, headers, options) {
super(stream, headers, options);
Expand Down