ronantakizawa/codereview-bench huggingface dataset:
Repository: koajs/koa
File: lib/context.js (JavaScript)

Code before:
      originalUrl: this.originalUrl,
      req: '<original node req>',
      res: '<original node res>',
      socket: '<original node socket>',
    }
  },

  /**
   * Throw an error with `msg` and optional `status`
   * defaulting to 500. Note that these are user-level
   * errors, and the message may be exposed to the client.
   *
   *    this.throw(403)
   *    this.throw('name required', 400)
   *    this.throw(400, 'name required')
   *    this.throw('something exploded')
   *    this.throw(new Error('invalid'), 400);
   *    this.throw(400, new Error('invalid'));
   *
   * @param {String|Number|Error} err, msg or status
   * @param {String|Number|Error} err, msg or status
   * @api public
   */

  throw: function(msg, status){
    if ('number' == typeof msg) {
      var tmp = msg;
      msg = status || http.STATUS_CODES[tmp];
      status = tmp;
    }

    var err = msg instanceof Error ? msg : new Error(msg);
    err.status = status || err.status || 500;
    err.expose = 'number' == typeof err.status && http.STATUS_CODES[err.status] && err.status < 500;
    throw err;
  },

  /**
   * Default error handling.
   *
   * @param {Error} err
   * @api private
   */

  onerror: function(err){
    // don't do anything if there is no error.
    // this allows you to pass `this.onerror`
    // to node-style callbacks.
    if (null == err) return;

    assert(err instanceof Error, 'non-error thrown: ' + err);

Review comment (suggestion):
Very confuse if set `status` props like `this.throw("some error", 500, {status: 400, prop: true})`


Code after:
      originalUrl: this.originalUrl,
      req: '<original node req>',
      res: '<original node res>',
      socket: '<original node socket>',
    }
  },

  /**
   * Throw an error with `msg` and optional `status`
   * defaulting to 500. Note that these are user-level
   * errors, and the message may be exposed to the client.
   *
   *    this.throw(403)
   *    this.throw('name required', 400)
   *    this.throw(400, 'name required')
   *    this.throw('something exploded')
   *    this.throw(new Error('invalid'), 400);
   *    this.throw(400, new Error('invalid'));
   *
   * @param {String|Number|Error} err, msg or status
   * @param {String|Number|Error} err, msg or status
   * @param {Object} [props]
   * @api public
   */

  throw: function(msg, status, props){
    if ('number' == typeof msg) {
      var tmp = msg;
      msg = status || http.STATUS_CODES[tmp];
      status = tmp;
    }

    var err = msg instanceof Error ? msg : new Error(msg);
    err.status = status || err.status || 500;
    err.expose = 'number' == typeof err.status && http.STATUS_CODES[err.status] && err.status < 500;

    if (props) {
      for (var key in props) err[key] = props[key];
    }

    throw err;
  },

  /**
   * Default error handling.
   *
   * @param {Error} err
   * @api private
   */

  onerror: function(err){
Repository: koajs/koa
File: lib/context.js (JavaScript)

Code before:
      originalUrl: this.originalUrl,
      req: '<original node req>',
      res: '<original node res>',
      socket: '<original node socket>',
    }
  },

  /**
   * Throw an error with `msg` and optional `status`
   * defaulting to 500. Note that these are user-level
   * errors, and the message may be exposed to the client.
   *
   *    this.throw(403)
   *    this.throw('name required', 400)
   *    this.throw(400, 'name required')
   *    this.throw('something exploded')
   *    this.throw(new Error('invalid'), 400);
   *    this.throw(400, new Error('invalid'));
   *
   * @param {String|Number|Error} err, msg or status
   * @param {String|Number|Error} err, msg or status
   * @api public
   */

  throw: function(msg, status){
    if ('number' == typeof msg) {
      var tmp = msg;
      msg = status || http.STATUS_CODES[tmp];
      status = tmp;
    }

    var err = msg instanceof Error ? msg : new Error(msg);
    err.status = status || err.status || 500;
    err.expose = 'number' == typeof err.status && http.STATUS_CODES[err.status] && err.status < 500;
    throw err;
  },

  /**
   * Default error handling.
   *
   * @param {Error} err
   * @api private
   */

  onerror: function(err){
    // don't do anything if there is no error.
    // this allows you to pass `this.onerror`
    // to node-style callbacks.
    if (null == err) return;

    assert(err instanceof Error, 'non-error thrown: ' + err);

Review comment (suggestion):
I wont do that...but if some one, who don't know `this.throw` will this `status` to error, want to set `status` prop to the error...


Code after:
      originalUrl: this.originalUrl,
      req: '<original node req>',
      res: '<original node res>',
      socket: '<original node socket>',
    }
  },

  /**
   * Throw an error with `msg` and optional `status`
   * defaulting to 500. Note that these are user-level
   * errors, and the message may be exposed to the client.
   *
   *    this.throw(403)
   *    this.throw('name required', 400)
   *    this.throw(400, 'name required')
   *    this.throw('something exploded')
   *    this.throw(new Error('invalid'), 400);
   *    this.throw(400, new Error('invalid'));
   *
   * @param {String|Number|Error} err, msg or status
   * @param {String|Number|Error} err, msg or status
   * @param {Object} [props]
   * @api public
   */

  throw: function(msg, status, props){
    if ('number' == typeof msg) {
      var tmp = msg;
      msg = status || http.STATUS_CODES[tmp];
      status = tmp;
    }

    var err = msg instanceof Error ? msg : new Error(msg);
    err.status = status || err.status || 500;
    err.expose = 'number' == typeof err.status && http.STATUS_CODES[err.status] && err.status < 500;

    if (props) {
      for (var key in props) err[key] = props[key];
    }

    throw err;
  },

  /**
   * Default error handling.
   *
   * @param {Error} err
   * @api private
   */

  onerror: function(err){
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:
  },

  /**
   * Get response status code.
   *
   * @return {Number}
   * @api public
   */

  get status() {
    return this.res.statusCode;
  },

  /**
   * Set response status code.
   *
   * @param {Number} code
   * @api public
   */

  set status(code) {
    assert('number' == typeof code, 'status code must be a number');
    assert(statuses[code], 'invalid status code: ' + code);
    this._explicitStatus = true;
    this.res.statusCode = code;
    this.res.statusMessage = statuses[code];
    if (this.body && statuses.empty[code]) this.body = null;
  },

  /**
   * Get response body.
   *
   * @return {Mixed}
   * @api public
   */

  get body() {
    return this._body;
  },

  /**
   * Set response body.
   *
   * @param {String|Buffer|Object|Stream} val
   * @api public
   */

  set body(val) {
    var original = this._body;
    this._body = val;


Review comment (suggestion):
yup. you can see it request by curl

```
curl http://127.0.0.1:3000/ -i
HTTP/1.1 700 Custom Status
```


Code after:
  },

  /**
   * Get response status code.
   *
   * @return {Number}
   * @api public
   */

  get status() {
    return this.res.statusCode;
  },

  /**
   * Set response status code.
   *
   * @param {Number} code
   * @api public
   */

  set status(code) {
    assert('number' == typeof code, 'status code must be a number');
    assert(statuses[code], 'invalid status code: ' + code);
    this._explicitStatus = true;
    this.res.statusCode = code;
    this.res.statusMessage = statuses[code];
    if (this.body && statuses.empty[code]) this.body = null;
  },

  /**
   * Get response status message
   *
   * @return {String}
   * @api public
   */

  get message() {
    return this.res.statusMessage || statuses[this.status];
  },

  /**
   * Set response status message
   *
   * @param {String} msg
   * @api public
   */

  set message(msg) {
    this.res.statusMessage = msg;
  },
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:
  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
  },

  /**
   * Inspect implementation.
   *
   * @return {Object}
   * @api public
   */

  inspect: function(){
    if (!this.res) return;
    var o = this.toJSON();
    o.body = this.body;
    return o;
  },

  /**
   * Return JSON representation.
   *
   * @return {Object}
   * @api public
   */

  toJSON: function(){
    return {
      status: this.status,
      message: this.message,

Review comment (suggestion):
not a big deal but I'd just do:

``` js
var prev = this.get(field);

if (prev) {
  val = Array.isArray(prev)
    ? prev.concat(val)
    : [prev].concat(val);
}

return this.set(field, val);
```

unless I'm missing a case there, otherwise LGTM!


Code after:
  /**
   * Append additional header `field` with value `val`.
   *
   * Examples:
   *
   *    this.append('Link', ['<http://localhost/>', '<http://localhost:3000/>']);
   *    this.append('Set-Cookie', 'foo=bar; Path=/; HttpOnly');
   *    this.append('Warning', '199 Miscellaneous warning');
   *
   * @param {String} field
   * @param {String|Array} val
   * @api public
   */

  append: function(field, val){
    var prev = this.get(field);
    var value = val;
    if (prev) {
      // concat the new and prev vals
      value = Array.isArray(prev) ? prev.concat(val)
        : Array.isArray(val) ? [prev].concat(val)
        : [prev, val];
    }

    return this.set(field, value);
  },

  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:
  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
  },

  /**
   * Inspect implementation.
   *
   * @return {Object}
   * @api public
   */

  inspect: function(){
    if (!this.res) return;
    var o = this.toJSON();
    o.body = this.body;
    return o;
  },

  /**
   * Return JSON representation.
   *
   * @return {Object}
   * @api public
   */

  toJSON: function(){
    return {
      status: this.status,
      message: this.message,

Review comment (suggestion):
I think @dead-horse used an intermediate variable because there is a perf hit in v8 when reassigning the value of a function argument (though I think that may only be the case when also accessing `arguments` within the same function, I'm not sure).


Code after:
  /**
   * Append additional header `field` with value `val`.
   *
   * Examples:
   *
   *    this.append('Link', ['<http://localhost/>', '<http://localhost:3000/>']);
   *    this.append('Set-Cookie', 'foo=bar; Path=/; HttpOnly');
   *    this.append('Warning', '199 Miscellaneous warning');
   *
   * @param {String} field
   * @param {String|Array} val
   * @api public
   */

  append: function(field, val){
    var prev = this.get(field);
    var value = val;
    if (prev) {
      // concat the new and prev vals
      value = Array.isArray(prev) ? prev.concat(val)
        : Array.isArray(val) ? [prev].concat(val)
        : [prev, val];
    }

    return this.set(field, value);
  },

  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:
  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
  },

  /**
   * Inspect implementation.
   *
   * @return {Object}
   * @api public
   */

  inspect: function(){
    if (!this.res) return;
    var o = this.toJSON();
    o.body = this.body;
    return o;
  },

  /**
   * Return JSON representation.
   *
   * @return {Object}
   * @api public
   */

  toJSON: function(){
    return {
      status: this.status,
      message: this.message,

Review comment (suggestion):
Oh, it's a direct port :) I'd say that's why I choose to use an intermediate :) Just because I didn't want to be sure it was a perf hit, haha.


Code after:
  /**
   * Append additional header `field` with value `val`.
   *
   * Examples:
   *
   *    this.append('Link', ['<http://localhost/>', '<http://localhost:3000/>']);
   *    this.append('Set-Cookie', 'foo=bar; Path=/; HttpOnly');
   *    this.append('Warning', '199 Miscellaneous warning');
   *
   * @param {String} field
   * @param {String|Array} val
   * @api public
   */

  append: function(field, val){
    var prev = this.get(field);
    var value = val;
    if (prev) {
      // concat the new and prev vals
      value = Array.isArray(prev) ? prev.concat(val)
        : Array.isArray(val) ? [prev].concat(val)
        : [prev, val];
    }

    return this.set(field, value);
  },

  /**
   * Remove header `field`.
   *
   * @param {String} name
   * @api public
   */

  remove: function(field){
    this.res.removeHeader(field);
  },

  /**
   * Checks if the request is writable.
   * Tests for the existence of the socket
   * as node sometimes does not set it.
   *
   * @return {Boolean}
   * @api private
   */

  get writable() {
    var socket = this.res.socket;
    if (!socket) return false;
    return socket.writable;
Repository: koajs/koa
File: lib/request.js (JavaScript)

Code before:
  get idempotent() {
    var methods = ['GET', 'HEAD', 'PUT', 'DELETE', 'OPTIONS', 'TRACE'];
    return !!~methods.indexOf(this.method);
  },

  /**
   * Return the request socket.
   *
   * @return {Connection}
   * @api public
   */

  get socket() {
    return this.req.socket;
  },

  /**
   * Get the charset when present or undefined.
   *
   * @return {String}
   * @api public
   */

  get charset() {
    var type = this.get('Content-Type');
    if (!type) return '';

    return contentType.parse(type).parameters.charset || '';
  },

  /**
   * Return parsed Content-Length when present.
   *
   * @return {Number}
   * @api public
   */

  get length() {
    var len = this.get('Content-Length');
    if (len == '') return;
    return ~~len;
  },

  /**
   * Return the protocol string "http" or "https"
   * when requested with TLS. When the proxy setting
   * is enabled the "X-Forwarded-Proto" header
   * field will be trusted. If you're running behind
   * a reverse proxy that supplies https for you this
   * may be enabled.
   *

Review comment (suggestion):
This line can be removed since this codepath can be handled implicitly by the `try/catch` block below.

Code after:
  get idempotent() {
    var methods = ['GET', 'HEAD', 'PUT', 'DELETE', 'OPTIONS', 'TRACE'];
    return !!~methods.indexOf(this.method);
  },

  /**
   * Return the request socket.
   *
   * @return {Connection}
   * @api public
   */

  get socket() {
    return this.req.socket;
  },

  /**
   * Get the charset when present or undefined.
   *
   * @return {String}
   * @api public
   */

  get charset() {
    var type = this.get('Content-Type');
    if (!type) return '';

    try {
      type = contentType.parse(type);
    } catch (e) {
      return '';
    }

    return type.parameters.charset || '';
  },

  /**
   * Return parsed Content-Length when present.
   *
   * @return {Number}
   * @api public
   */

  get length() {
    var len = this.get('Content-Length');
    if (len == '') return;
    return ~~len;
  },

  /**
   * Return the protocol string "http" or "https"
Repository: koajs/koa
File: lib/request.js (JavaScript)

Code before:
   * Parse the "Host" header field host
   * and support X-Forwarded-Host when a
   * proxy is enabled.
   *
   * @return {String} hostname:port
   * @api public
   */

  get host() {
    const proxy = this.app.proxy;
    let host = proxy && this.get('X-Forwarded-Host');
    host = host || this.get('Host');
    if (!host) return '';
    return host.split(/\s*,\s*/)[0];
  },

  /**
   * Parse the "Host" header field hostname
   * and support X-Forwarded-Host when a
   * proxy is enabled.
   *
   * @return {String} hostname
   * @api public
   */

  get hostname() {
    if (!this.parsedUrl) {
      const host = this.host;
      if (!host) return '';
      this.parsedUrl = new URL(`${this.protocol}://${this.host}`);
    }
    return this.parsedUrl.hostname;
  },

  /**
   * Check if the request is fresh, aka
   * Last-Modified and/or the ETag
   * still match.
   *
   * @return {Boolean}
   * @api public
   */

  get fresh() {
    const method = this.method;
    const s = this.ctx.status;

    // GET or HEAD for weak freshness validation only
    if ('GET' != method && 'HEAD' != method) return false;

    // 2xx or 304 as per rfc2616 14.26

Review comment (suggestion):
This needs some adjusting - the code path is weird and for some reason I'm not using `host` whilst also assigning `const host = this.host` (relic from the past). I'll clean this up later today.

Code after:
   * Parse the "Host" header field host
   * and support X-Forwarded-Host when a
   * proxy is enabled.
   *
   * @return {String} hostname:port
   * @api public
   */

  get host() {
    const proxy = this.app.proxy;
    let host = proxy && this.get('X-Forwarded-Host');
    host = host || this.get('Host');
    if (!host) return '';
    return host.split(/\s*,\s*/)[0];
  },

  /**
   * Parse the "Host" header field hostname
   * and support X-Forwarded-Host when a
   * proxy is enabled.
   *
   * @return {String} hostname
   * @api public
   */

  get hostname() {
    const host = this.host;
    if (!host) return '';
    if ('[' == host[0]) return this.URL.hostname || ''; // IPv6
    return host.split(':')[0];
  },

  /**
   * Get WHATWG parsed URL.
   * Lazily memoized.
   *
   * @return {URL|Object}
   * @api public
   */

  get URL() {
    if (!this.memoizedURL) {
      const protocol = this.protocol;
      const host = this.host;
      const originalUrl = this.originalUrl || ''; // avoid undefined in template string
      try {
        this.memoizedURL = new URL(`${protocol}://${host}${originalUrl}`);
      } catch (err) {
        this.memoizedURL = Object.create(null);
      }
    }
Repository: koajs/koa
File: lib/request.js (JavaScript)

Code before:
   *
   * @return {String} hostname
   * @api public
   */

  get hostname() {
    const host = this.host;
    if (!host) return '';
    if ('[' == host[0]) return this.URL.hostname; // IPv6
    return host.split(':')[0];
  },

  /**
   * Get WHATWG parsed URL.
   * Lazily memoized.
   *
   * @return {URL}
   * @api public
   */

  get URL() {
    if (!this.memoizedURL) {
      const protocol = this.protocol;
      const host = this.host;
      const originalUrl = this.originalUrl || ''; // avoid undefined in template string
      this.memoizedURL = new URL(`${protocol}://${host}${originalUrl}`);
    }
    return this.memoizedURL;
  },

  /**
   * Check if the request is fresh, aka
   * Last-Modified and/or the ETag
   * still match.
   *
   * @return {Boolean}
   * @api public
   */

  get fresh() {
    const method = this.method;
    const s = this.ctx.status;

    // GET or HEAD for weak freshness validation only
    if ('GET' != method && 'HEAD' != method) return false;

    // 2xx or 304 as per rfc2616 14.26
    if ((s >= 200 && s < 300) || 304 == s) {
      return fresh(this.header, this.ctx.response.header);
    }


Review comment (bug):
URL will throw TypeError if something's off.
Koa generally has `value || ''` type of solutions in most cases.

Maybe wrap this in a try-catch and memoize empty object so that `this.URL.hostname` when malformed does not throw/crash the entire app?

Code after:
   *
   * @return {String} hostname
   * @api public
   */

  get hostname() {
    const host = this.host;
    if (!host) return '';
    if ('[' == host[0]) return this.URL.hostname || ''; // IPv6
    return host.split(':')[0];
  },

  /**
   * Get WHATWG parsed URL.
   * Lazily memoized.
   *
   * @return {URL|Object}
   * @api public
   */

  get URL() {
    if (!this.memoizedURL) {
      const protocol = this.protocol;
      const host = this.host;
      const originalUrl = this.originalUrl || ''; // avoid undefined in template string
      try {
        this.memoizedURL = new URL(`${protocol}://${host}${originalUrl}`);
      } catch (err) {
        this.memoizedURL = Object.create(null);
      }
    }
    return this.memoizedURL;
  },

  /**
   * Check if the request is fresh, aka
   * Last-Modified and/or the ETag
   * still match.
   *
   * @return {Boolean}
   * @api public
   */

  get fresh() {
    const method = this.method;
    const s = this.ctx.status;

    // GET or HEAD for weak freshness validation only
    if ('GET' != method && 'HEAD' != method) return false;

    // 2xx or 304 as per rfc2616 14.26