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

http: add bytesWritten property #36964

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 9 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,15 @@ response.end();
Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

### `response.bytesWritten`
<!-- YAML
added: REPLACEME
-->

* {integer}

Number of bytes written to response body.

### `response.connection`
<!-- YAML
added: v0.3.0
Expand Down
9 changes: 9 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -3271,6 +3271,15 @@ message) to the response.
Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

### `response.bytesWritten`
<!-- YAML
added: REPLACEME
-->

* {integer}

Number of bytes written to response body.

#### `response.connection`
<!-- YAML
added: v8.4.0
Expand Down
11 changes: 11 additions & 0 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
const { CRLF, debug } = common;

const kCorked = Symbol('corked');
const kBytesWritten = Symbol('bytesWritten');

const nop = FunctionPrototype;

Expand Down Expand Up @@ -132,6 +133,7 @@ function OutgoingMessage() {
this.socket = null;
this._header = null;
this[kOutHeaders] = null;
this[kBytesWritten] = 0;

this._keepAliveTimeout = 0;

Expand All @@ -140,6 +142,12 @@ function OutgoingMessage() {
ObjectSetPrototypeOf(OutgoingMessage.prototype, Stream.prototype);
ObjectSetPrototypeOf(OutgoingMessage, Stream);

ObjectDefineProperty(OutgoingMessage.prototype, 'bytesWritten', {
get() {
return this[kBytesWritten];
}
});
Copy link
Member Author

Choose a reason for hiding this comment

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

Could this break userland which extend e.g. ServerResponse and add this themselves?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure. Maybe some web framework listed in https://github.com/nodejs/citgm/blob/main/lib/lookup.json? I think we could also try some HTTP clients like got.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a setter as well to try and avoid breaking


ObjectDefineProperty(OutgoingMessage.prototype, 'writableFinished', {
get() {
return (
Expand Down Expand Up @@ -684,6 +692,9 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
}

const ret = write_(this, chunk, encoding, callback, false);

this[kBytesWritten] += Buffer.byteLength(chunk, encoding);

if (!ret)
this[kNeedDrain] = true;
return ret;
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ class Http2ServerResponse extends Stream {
return this.headersSent;
}

get bytesWritten() {
return this.stream.bytesWritten;
}

get writableEnded() {
const state = this[kState];
return state.ending;
Expand Down
19 changes: 16 additions & 3 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ const { onServerStream,
Http2ServerRequest,
Http2ServerResponse,
} = require('internal/http2/compat');
const { Buffer } = require('buffer');

const {
assertIsObject,
Expand Down Expand Up @@ -1906,7 +1907,8 @@ class Http2Stream extends Duplex {
rstCode: NGHTTP2_NO_ERROR,
writeQueueSize: 0,
trailersReady: false,
endAfterHeaders: false
endAfterHeaders: false,
bytesWritten: 0
ronag marked this conversation as resolved.
Show resolved Hide resolved
};

// Fields used by the compat API to avoid megamorphisms.
Expand Down Expand Up @@ -1960,6 +1962,10 @@ class Http2Stream extends Duplex {
return `Http2Stream ${format(obj)}`;
}

get bytesWritten() {
return this[kState].bytesWritten;
}

get bufferSize() {
// `bufferSize` properties of `net.Socket` are `undefined` when
// their `_handle` are falsy. Here we avoid the behavior.
Expand Down Expand Up @@ -2107,14 +2113,21 @@ class Http2Stream extends Duplex {
shutdownWritable.call(this, endCheckCallback);
});

if (writev)
if (writev) {
req = writevGeneric(this, data, writeCallback);
else
} else {
req = writeGeneric(this, data, encoding, writeCallback);
}
ronag marked this conversation as resolved.
Show resolved Hide resolved

trackWriteState(this, req.bytes);
}

write(data, encoding, cb) {
const ret = super.write(data, encoding, cb);
this[kState].bytesWritten += Buffer.byteLength(data, encoding);
return ret;
}

_write(data, encoding, cb) {
this[kWriteGeneric](false, data, encoding, cb);
}
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-http-byteswritten.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,21 @@ const httpServer = http.createServer(common.mustCall(function(req, res) {

// Write 1.5mb to cause some requests to buffer
// Also, mix up the encodings a bit.
let bytesWritten = 0;
const chunk = '7'.repeat(1024);
const bchunk = Buffer.from(chunk);
for (let i = 0; i < 1024; i++) {
res.write(chunk);
bytesWritten += chunk.length;
res.write(bchunk);
bytesWritten += bchunk.length;
res.write(chunk, 'hex');
bytesWritten += Buffer.byteLength(chunk, 'hex');

}
// Get .bytesWritten while buffer is not empty
assert(res.connection.bytesWritten > 0);
assert.strictEqual(res.bytesWritten, bytesWritten);

res.end(body);
}));
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http2-buffersize.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ const { once } = require('events');
for (let j = 0; j < kSockets; j += 1) {
const stream = client.request({ ':method': 'POST' });
stream.on('data', () => {});

let bytesWritten = 0;
for (let i = 0; i < kTimes; i += 1) {
stream.write(Buffer.allocUnsafe(kBufferSize), mustSucceed());
bytesWritten += kBufferSize;
assert.strictEqual(stream.bytesWritten, bytesWritten);
const expectedSocketBufferSize = kBufferSize * (i + 1);
assert.strictEqual(stream.bufferSize, expectedSocketBufferSize);
}
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-http2-compat-serverresponse-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const assert = require('assert');
server.once('request', mustCall((request, response) => {
// response.write() returns true
assert(response.write('muahaha', 'utf8', mustCall()));
assert.strictEqual(response.bytesWritten, Buffer.byteLength('muahaha'));

response.stream.close(0, mustCall(() => {
response.on('error', mustNotCall());
Expand Down