Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

use "finsh" event to write msg on response time #552

Closed
wants to merge 2 commits into from

4 participants

@youngjay

I found the "finish" event in node0.6.15/lib/http.js line 813

OutgoingMessage.prototype._finish = function() {
  assert(this.connection);
  if (this instanceof ServerResponse) {
    DTRACE_HTTP_SERVER_RESPONSE(this.connection);
  } else {
    assert(this instanceof ClientRequest);
    DTRACE_HTTP_CLIENT_REQUEST(this, this.connection);
  }
  this.emit('finish');
};

Is it better to use it than override the original "res.end" ?

@tj
Owner
tj commented

"finish" is not publish API but it should be ok to use, we can always revert

lib/middleware/logger.js
@@ -143,14 +143,11 @@ exports = module.exports = function logger(options) {
stream.write(line + '\n', 'ascii');
// proxy end to output logging
} else {
- var end = res.end;
- res.end = function(chunk, encoding){
- res.end = end;
- res.end(chunk, encoding);
- var line = fmt(exports, req, res);
- if (null == line) return;
- stream.write(line + '\n', 'ascii');
- };
+ res.on('finish', function(chunk, encoding) {
+ var line = fmt(exports, req, res);
+ if (null == line) return;
+ stream.write(line + '\n', 'ascii');
+ });
@tj Owner
tj added a note

your indentation is all wonky here though, 2 spaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@youngjay

converted the intendations from 1 tab to 2 spaces

@dylanahsmith

"finish" is not publish API but it should be ok to use, we can always revert

@visionmedia, it looks like the 'finish' event it is now a part of the public Stream API.

@dylanahsmith dylanahsmith commented on the diff
lib/middleware/logger.js
@@ -143,14 +143,11 @@ exports = module.exports = function logger(options) {
stream.write(line + '\n', 'ascii');
// proxy end to output logging
} else {
- var end = res.end;
- res.end = function(chunk, encoding){
- res.end = end;
- res.end(chunk, encoding);
+ res.on('finish', function(chunk, encoding) {

The finish event isn't emitted with any parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dylanahsmith

Minor comment, otherwise :+1:

This will also make it easier to just add another listener for the 'close' event to further fix issue #321

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 27, 2012
  1. @youngjay
Commits on Apr 28, 2012
  1. @youngjay
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 5 deletions.
  1. +2 −5 lib/middleware/logger.js
View
7 lib/middleware/logger.js
@@ -143,14 +143,11 @@ exports = module.exports = function logger(options) {
stream.write(line + '\n', 'ascii');
// proxy end to output logging
} else {
- var end = res.end;
- res.end = function(chunk, encoding){
- res.end = end;
- res.end(chunk, encoding);
+ res.on('finish', function(chunk, encoding) {

The finish event isn't emitted with any parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
var line = fmt(exports, req, res);
if (null == line) return;
stream.write(line + '\n', 'ascii');
- };
+ });
}
Something went wrong with that request. Please try again.