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

Code before:

'use strict';

/**
 * Module dependencies.
 */

const isGeneratorFunction = require('is-generator-function');
const debug = require('debug')('koa:application');
const onFinished = require('on-finished');
const response = require('./response');
const compose = require('koa-compose');
const isJSON = require('koa-is-json');
const context = require('./context');
const request = require('./request');
const statuses = require('statuses');
const Cookies = require('cookies');
const accepts = require('accepts');
const Emitter = require('events');
const util = require('util');
const Stream = require('stream');
const http = require('http');
const only = require('only');
const convert = require('koa-convert');
const deprecate = require('depd')('koa');
const util = require('util');

/**
 * Expose `Application` class.
 * Inherits from `Emitter.prototype`.
 */

module.exports = class Application extends Emitter {
  /**
   * Initialize a new `Application`.
   *
   * @api public
   */

  constructor() {
    super();

    this.proxy = false;
    this.middleware = [];
    this.subdomainOffset = 2;
    this.env = process.env.NODE_ENV || 'development';
    this.context = Object.create(context);
    this.request = Object.create(request);
    this.response = Object.create(response);
    if (util.inspect.custom) {
      this[util.inspect.custom] = this.inspect;

Review comment (refactor):
```
    /home/travis/build/koajs/koa/lib/application.js: Duplicate declaration "util"
        24 | const convert = require('koa-convert');
        25 | const deprecate = require('depd')('koa');
      > 26 | const util = require('util');
```

please fix this and good to merge. 👍 

Code after:

'use strict';

/**
 * Module dependencies.
 */

const isGeneratorFunction = require('is-generator-function');
const debug = require('debug')('koa:application');
const onFinished = require('on-finished');
const response = require('./response');
const compose = require('koa-compose');
const isJSON = require('koa-is-json');
const context = require('./context');
const request = require('./request');
const statuses = require('statuses');
const Cookies = require('cookies');
const accepts = require('accepts');
const Emitter = require('events');
const util = require('util');
const Stream = require('stream');
const http = require('http');
const only = require('only');
const convert = require('koa-convert');
const deprecate = require('depd')('koa');

/**
 * Expose `Application` class.
 * Inherits from `Emitter.prototype`.
 */

module.exports = class Application extends Emitter {
  /**
   * Initialize a new `Application`.
   *
   * @api public
   */

  constructor() {
    super();

    this.proxy = false;
    this.middleware = [];
    this.subdomainOffset = 2;
    this.env = process.env.NODE_ENV || 'development';
    this.context = Object.create(context);
    this.request = Object.create(request);
    this.response = Object.create(response);
    if (util.inspect.custom) {
      this[util.inspect.custom] = this.inspect;
    }
Repository: koajs/koa
File: lib/request.js (JavaScript)

Code before:
   * `["ferrets", "tobi"]`.
   * If `app.subdomainOffset` is 3, this.subdomains is `["tobi"]`.
   *
   * @return {Array}
   * @api public
   */

  get subdomains() {
    const offset = this.app.subdomainOffset;
    const hostname = this.hostname;
    if (net.isIP(hostname)) return [];
    return hostname
      .split('.')
      .reverse()
      .slice(offset);
  },

  /**
   * Get accept object.
   * Lazily memoized.
   *
   * @return {Object}
   * @api public
   */
  get accept() {
    return this._accept || (this._accept = accepts(this.req));
  },

  /**
   * Check if the given `type(s)` is acceptable, returning
   * the best match when true, otherwise `false`, in which
   * case you should respond with 406 "Not Acceptable".
   *
   * The `type` value may be a single mime type string
   * such as "application/json", the extension name
   * such as "json" or an array `["json", "html", "text/plain"]`. When a list
   * or array is given the _best_ match, if any is returned.
   *
   * Examples:
   *
   *     // Accept: text/html
   *     this.accepts('html');
   *     // => "html"
   *
   *     // Accept: text/*, application/json
   *     this.accepts('html');
   *     // => "html"
   *     this.accepts('text/html');
   *     // => "text/html"
   *     this.accepts('json', 'text');
   *     // => "json"

Review comment (suggestion):
maybe you should add `set accept()` to avoid breaking change.

Code after:
   * If `app.subdomainOffset` is not set, this.subdomains is
   * `["ferrets", "tobi"]`.
   * If `app.subdomainOffset` is 3, this.subdomains is `["tobi"]`.
   *
   * @return {Array}
   * @api public
   */

  get subdomains() {
    const offset = this.app.subdomainOffset;
    const hostname = this.hostname;
    if (net.isIP(hostname)) return [];
    return hostname
      .split('.')
      .reverse()
      .slice(offset);
  },

  /**
   * Get accept object.
   * Lazily memoized.
   *
   * @return {Object}
   * @api private
   */
  get accept() {
    return this._accept || (this._accept = accepts(this.req));
  },

  /**
   * Set accept object.
   *
   * @param {Object}
   * @api private
   */
  set accept(obj) {
    return this._accept = obj;
  },

  /**
   * Check if the given `type(s)` is acceptable, returning
   * the best match when true, otherwise `false`, in which
   * case you should respond with 406 "Not Acceptable".
   *
   * The `type` value may be a single mime type string
   * such as "application/json", the extension name
   * such as "json" or an array `["json", "html", "text/plain"]`. When a list
   * or array is given the _best_ match, if any is returned.
   *
   * Examples:
   *
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:

  get(field) {
    return this.header[field.toLowerCase()] || '';
  },

  /**
   * Set header `field` to `val`, or pass
   * an object of header fields.
   *
   * Examples:
   *
   *    this.set('Foo', ['bar', 'baz']);
   *    this.set('Accept', 'application/json');
   *    this.set({ Accept: 'text/plain', 'X-API-Key': 'tobi' });
   *
   * @param {String|Object|Array} field
   * @param {String} val
   * @api public
   */

  set(field, val) {
    if (this.headerSent) return;

    if (2 == arguments.length) {
      // maybe we can drop support of undefined in the next major version
      if (val === undefined) val = 'undefined';
      this.res.setHeader(field, val);
    } else {
      for (const key in field) {
        this.set(key, field[key]);
      }
    }
  },

  /**
   * 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(field, val) {

Review comment (suggestion):
other type of values will be stringify in `res.setHeader`

Code after:

  get(field) {
    return this.header[field.toLowerCase()] || '';
  },

  /**
   * Set header `field` to `val`, or pass
   * an object of header fields.
   *
   * Examples:
   *
   *    this.set('Foo', ['bar', 'baz']);
   *    this.set('Accept', 'application/json');
   *    this.set({ Accept: 'text/plain', 'X-API-Key': 'tobi' });
   *
   * @param {String|Object|Array} field
   * @param {String} val
   * @api public
   */

  set(field, val) {
    if (this.headerSent) return;

    if (2 == arguments.length) {
      if (Array.isArray(val)) val = val.map(v => typeof v === 'string' ? v : String(v));
      else if (typeof val !== 'string') val = String(val);
      this.res.setHeader(field, val);
    } else {
      for (const key in field) {
        this.set(key, field[key]);
      }
    }
  },

  /**
   * 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(field, val) {
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type');

    // string
    if ('string' === typeof val) {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text';
      this.length = Buffer.byteLength(val);
      return;
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin';
      this.length = val.length;
      return;
    }

    // stream
    if ('function' === typeof val.pipe) {
      onFinish(this.res, destroy.bind(null, val));
      ensureErrorHandler(val, err => this.ctx.onerror(err));

      // overwriting
      if (null != original && original !== val) this.remove('Content-Length');

      if (setType) this.type = 'bin';
      return;
    }

    // json
    this.remove('Content-Length');
    this.type = 'json';
  },

  /**
   * Set Content-Length field to `n`.
   *
   * @param {Number} n
   * @api public
   */

  set length(n) {
    this.set('Content-Length', n);
  },

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

Review comment (suggestion):
```suggestion
      if (null !== original && original !== val) this.remove('Content-Length');
```


Code after:
    if (!this._explicitStatus) this.status = 200;

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type');

    // string
    if ('string' === typeof val) {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text';
      this.length = Buffer.byteLength(val);
      return;
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin';
      this.length = val.length;
      return;
    }

    // stream
    if ('function' === typeof val.pipe) {
      onFinish(this.res, destroy.bind(null, val));
      ensureErrorHandler(val, err => this.ctx.onerror(err));

      // overwriting
      if (null != original && original !== val) this.remove('Content-Length');

      if (setType) this.type = 'bin';
      return;
    }

    // json
    this.remove('Content-Length');
    this.type = 'json';
  },

  /**
   * Set Content-Length field to `n`.
   *
   * @param {Number} n
   * @api public
   */

  set length(n) {
    this.set('Content-Length', n);
  },

  /**
   * Return parsed response Content-Length when present.
   *
   * @return {Number}
Repository: koajs/koa
File: lib/request.js (JavaScript)

Code before:

  /**
   * Set the search string. Same as
   * request.querystring= but included for ubiquity.
   *
   * @param {String} str
   * @api public
   */

  set search(str) {
    this.querystring = str;
  },

  /**
   * 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}

Review comment (security):
https://github.com/nodejs/node/blob/e7f710c1d4729890806610e5450d7a0edfb4ac11/lib/internal/http2/compat.js#L358

use `this.req. authority` will be better.

Code after:

  /**
   * Set the search string. Same as
   * request.querystring= but included for ubiquity.
   *
   * @param {String} str
   * @api public
   */

  set search(str) {
    this.querystring = str;
  },

  /**
   * 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');
    if (this.req.httpVersionMajor >= 2) host = this.get(':authority');
    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.
   *
Repository: koajs/koa
File: lib/application.js (JavaScript)

Code before:

/**
 * Response helper.
 */

function respond(ctx) {
  // allow bypassing koa
  if (false === ctx.respond) return;

  if (!ctx.writable) return;

  const res = ctx.res;
  let body = ctx.body;
  const code = ctx.status;

  // ignore body
  if (statuses.empty[code]) {
    // strip headers
    ctx.body = null;
    return res.end();
  }

  if ('HEAD' === ctx.method) {
    if (!res.headersSent && !ctx.response.has('Content-Length')) {
      const { length } = ctx.response;
      if (Number.isInteger(length)) { ctx.length = length; }
    }
    return res.end();
  }

  // status body
  if (null == body) {
    if (ctx.req.httpVersionMajor >= 2) {
      body = String(code);
    } else {
      body = ctx.message || String(code);
    }
    if (!res.headersSent) {
      ctx.type = 'text';
      ctx.length = Buffer.byteLength(body);
    }
    return res.end(body);
  }

  // responses
  if (Buffer.isBuffer(body)) return res.end(body);
  if ('string' == typeof body) return res.end(body);
  if (body instanceof Stream) return body.pipe(res);

  // body: json
  body = JSON.stringify(body);

Review comment (suggestion):
I'd like to remove `{}` in single line, others LGTM

Code after:

/**
 * Response helper.
 */

function respond(ctx) {
  // allow bypassing koa
  if (false === ctx.respond) return;

  if (!ctx.writable) return;

  const res = ctx.res;
  let body = ctx.body;
  const code = ctx.status;

  // ignore body
  if (statuses.empty[code]) {
    // strip headers
    ctx.body = null;
    return res.end();
  }

  if ('HEAD' === ctx.method) {
    if (!res.headersSent && !ctx.response.has('Content-Length')) {
      const { length } = ctx.response;
      if (Number.isInteger(length)) ctx.length = length;
    }
    return res.end();
  }

  // status body
  if (null == body) {
    if (ctx.req.httpVersionMajor >= 2) {
      body = String(code);
    } else {
      body = ctx.message || String(code);
    }
    if (!res.headersSent) {
      ctx.type = 'text';
      ctx.length = Buffer.byteLength(body);
    }
    return res.end(body);
  }

  // responses
  if (Buffer.isBuffer(body)) return res.end(body);
  if ('string' == typeof body) return res.end(body);
  if (body instanceof Stream) return body.pipe(res);

  // body: json
  body = JSON.stringify(body);
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:
    // set the status
    if (!this._explicitStatus) this.status = 200;

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type');

    // string
    if ('string' == typeof val) {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text';
      this.length = Buffer.byteLength(val);
      return;
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin';
      this.length = val.length;
      return;
    }

    // stream
    if (val instanceof Stream) {
      onFinish(this.res, destroy.bind(null, val));

      if (original != val) {
        val.once('error', err => this.ctx.onerror(err));
        // overwriting
        if (null != original) this.remove('Content-Length');
      }

      if (setType) this.type = 'bin';
      return;
    }

    // json
    this.remove('Content-Length');
    this.type = 'json';
  },

  /**
   * Set Content-Length field to `n`.
   *
   * @param {Number} n
   * @api public
   */

  set length(n) {
    this.set('Content-Length', n);
  },

  /**

Review comment (suggestion):
The two implementations seem to be not equal. We should only add `error` event listener when the stream has no other error event listener, which aim to avoid uncaught exception.

Also IMO koa's code philosophy is to keep code simple, so we don't treat 'one-liner module' as a bad thing.

Code after:

    // set the status
    if (!this._explicitStatus) this.status = 200;

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type');

    // string
    if ('string' === typeof val) {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text';
      this.length = Buffer.byteLength(val);
      return;
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin';
      this.length = val.length;
      return;
    }

    // stream
    if (val instanceof Stream) {
      onFinish(this.res, destroy.bind(null, val));
      if (original != val) {
        val.once('error', err => this.ctx.onerror(err));
        // overwriting
        if (null != original) this.remove('Content-Length');
      }

      if (setType) this.type = 'bin';
      return;
    }

    // json
    this.remove('Content-Length');
    this.type = 'json';
  },

  /**
   * Set Content-Length field to `n`.
   *
   * @param {Number} n
   * @api public
   */

  set length(n) {
    this.set('Content-Length', n);
  },

  /**
Repository: koajs/koa
File: __tests__/context/throw.test.js (JavaScript)

Code before:

    try {
      ctx.throw(err)
    } catch (err) {
      assert.strictEqual(err.status, 500)
      assert.strictEqual(err.message, 'test')
      assert.strictEqual(err.expose, false)
    }
  })
})

describe('ctx.throw(err, status)', () => {
  it('should throw the error and set .status', () => {
    const ctx = context()
    const error = new Error('test')

    try {
      ctx.throw(error, 422)
    } catch (err) {
      assert.strictEqual(err.status, 422)
      assert.strictEqual(err.message, 'test')
      assert.strictEqual(err.expose, true)
    }
  })
})

describe('ctx.throw(status, err)', () => {
  it('should throw the error and set .status', () => {
    const ctx = context()
    const error = new Error('test')

    try {
      ctx.throw(422, error)
    } catch (err) {
      assert.strictEqual(err.status, 422)
      assert.strictEqual(err.message, 'test')
      assert.strictEqual(err.expose, true)
    }
  })
})

describe('ctx.throw(msg, status)', () => {
  it('should throw an error', () => {
    const ctx = context()

    try {
      ctx.throw('name required', 400)
    } catch (err) {
      assert.strictEqual(err.message, 'name required')
      assert.strictEqual(err.status, 400)
      assert.strictEqual(err.expose, true)

Review comment (suggestion):
[The signature now should in the following order: status, errorOrMessage, properties.](https://github.com/jshttp/http-errors/tree/master?tab=readme-ov-file#createerrorstatus-error-properties)

Code after:

    try {
      ctx.throw(err)
    } catch (err) {
      assert.strictEqual(err.status, 500)
      assert.strictEqual(err.message, 'test')
      assert.strictEqual(err.expose, false)
    }
  })
})

describe('ctx.throw(status, err)', () => {
  it('should throw the error and set .status', () => {
    const ctx = context()
    const error = new Error('test')

    try {
      ctx.throw(422, error)
    } catch (err) {
      assert.strictEqual(err.status, 422)
      assert.strictEqual(err.message, 'test')
      assert.strictEqual(err.expose, true)
    }
  })
})

describe('ctx.throw(status, msg)', () => {
  it('should throw an error', () => {
    const ctx = context()

    try {
      ctx.throw(400, 'name required')
    } catch (err) {
      assert.strictEqual(err.message, 'name required')
      assert.strictEqual(400, err.status)
      assert.strictEqual(true, err.expose)
    }
  })
})

describe('ctx.throw(status)', () => {
  it('should throw an error', () => {
    const ctx = context()

    try {
      ctx.throw(400)
    } catch (err) {
      assert.strictEqual(err.message, 'Bad Request')
      assert.strictEqual(err.status, 400)
      assert.strictEqual(err.expose, true)
    }
Repository: koajs/koa
File: __tests__/application/index.test.js (JavaScript)

Code before:
const { describe, it } = require('node:test')
const assert = require('node:assert/strict')
const { once } = require('node:events')
const Koa = require('../..')

describe('app', () => {
  it('should handle socket errors', async () => {
    const app = new Koa()

    app.use((ctx) => {
      ctx.socket.destroy(new Error('boom'))
    })

    app.on('error', err => {
      assert.strictEqual(err.message, 'boom')
    })

    const server = app.listen()

    try {
      const req = require('http').get({
        port: server.address().port
      })
      req.on('error', () => {})

      await once(app, 'error')
    } finally {
      server.close()
    }
  })

  it('should set development env when NODE_ENV missing', () => {
    const NODE_ENV = process.env.NODE_ENV
    process.env.NODE_ENV = ''
    const app = new Koa()
    process.env.NODE_ENV = NODE_ENV
    assert.strictEqual(app.env, 'development')
  })

  it('should set env from the constructor', () => {
    const env = 'custom'
    const app = new Koa({ env })
    assert.strictEqual(app.env, env)
  })

  it('should set proxy flag from the constructor', () => {
    const proxy = true
    const app = new Koa({ proxy })
    assert.strictEqual(app.proxy, proxy)
  })


Review comment (suggestion):
```suggestion
      const [err] = await once(app, 'error')
      assert.strictEqual(err.message, 'boom')
```

Code after:
const { describe, it } = require('node:test')
const assert = require('node:assert/strict')
const { once } = require('node:events')
const Koa = require('../..')

describe('app', () => {
  it('should handle socket errors', async () => {
    const app = new Koa()
    let errorCaught = false

    app.use((ctx) => {
      ctx.socket.destroy(new Error('boom'))
    })

    app.on('error', err => {
      assert.strictEqual(err.message, 'boom')
      errorCaught = true
    })

    const server = app.listen()

    try {
      const req = require('http').get({
        port: server.address().port
      })
      req.on('error', () => {})

      const [err] = await once(app, 'error')
      assert.strictEqual(err.message, 'boom')
      assert.strictEqual(errorCaught, true)
    } finally {
      await server.close()
    }
  })

  it('should set development env when NODE_ENV missing', () => {
    const NODE_ENV = process.env.NODE_ENV
    process.env.NODE_ENV = ''
    const app = new Koa()
    process.env.NODE_ENV = NODE_ENV
    assert.strictEqual(app.env, 'development')
  })

  it('should set env from the constructor', () => {
    const env = 'custom'
    const app = new Koa({ env })
    assert.strictEqual(app.env, env)
  })

  it('should set proxy flag from the constructor', () => {
    const proxy = true