ronantakizawa/codereview-bench huggingface dataset:
Repository: honojs/hono
File: src/helper/streaming/sse.ts (TypeScript)

Code before:
  }
}

const setSSEHeaders = (context: Context) => {
  context.header('Transfer-Encoding', 'chunked')
  context.header('Content-Type', 'text/event-stream')
  context.header('Cache-Control', 'no-cache')
  context.header('Connection', 'keep-alive')
}

export const streamSSE = (
  c: Context,
  cb: (stream: SSEStreamingApi) => Promise<void>,
  onError?: (e: Error, stream: StreamingApi) => Promise<void>
) => {
  const { readable, writable } = new TransformStream()
  const stream = new SSEStreamingApi(writable, readable)
  ;(async () => {
    try {
      await cb(stream)
    } catch (e) {
      if (e instanceof Error && onError) {
        await onError(e, stream)
      }
    } finally {
      stream.close()
    }
  })()
  setSSEHeaders(c)
  return c.newResponse(stream.responseReadable)
}


Review comment (suggestion):
If `onError` was not set, how about outputting an error? This way the user may not know that an error has occurred.

Code after:
  }
}

const setSSEHeaders = (context: Context) => {
  context.header('Transfer-Encoding', 'chunked')
  context.header('Content-Type', 'text/event-stream')
  context.header('Cache-Control', 'no-cache')
  context.header('Connection', 'keep-alive')
}

export const streamSSE = (
  c: Context,
  cb: (stream: SSEStreamingApi) => Promise<void>,
  onError?: (e: Error, stream: SSEStreamingApi) => Promise<void>
) => {
  const { readable, writable } = new TransformStream()
  const stream = new SSEStreamingApi(writable, readable)
  ;(async () => {
    try {
      await cb(stream)
    } catch (e) {
      if (e instanceof Error && onError) {
        await onError(e, stream)
      } else {
        console.error(e)
      }
    } finally {
      stream.close()
    }
  })()
  setSSEHeaders(c)
  return c.newResponse(stream.responseReadable)
}
Repository: honojs/hono
File: src/helper/cookie/index.ts (TypeScript)

Code before:

  if (opt?.hostPrefix) {
    cookies.push(
      serialize('__Host-' + name, value, { secure: true, path: '/', domain: undefined, ...opt })
    )
  }

  if (!opt?.securePrefix && !opt?.hostPrefix) {
    cookies.push(serialize(name, value, { path: '/', ...opt }))
  }
  for (const cookie of cookies) {
    c.header('set-cookie', cookie, { append: true })
  }
}

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  const cookies = []
  if (opt?.securePrefix) {
    cookies.push(
      await serializeSigned('__Secure-' + name, value, secret, { path: '/', secure: true, ...opt })
    )
  }

  if (opt?.hostPrefix) {
    cookies.push(
      await serializeSigned('__Host-' + name, value, secret, {
        secure: true,
        path: '/',
        domain: undefined,
        ...opt,
      })
    )
  }

  if (!opt?.securePrefix && !opt?.hostPrefix) {
    cookies.push(await serializeSigned(name, value, secret, { path: '/', ...opt }))
  }
  for (const cookie of cookies) {
    c.header('set-cookie', cookie, { append: true })
  }
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}

Review comment (security):

```suggestion
      await serializeSigned('__Secure-' + name, value, secret, { path: '/', ...opt, secure: true })
```

Code after:
    cookie = serialize('__Host-' + name, value, {
      ...opt,
      path: '/',
      secure: true,
      domain: undefined,
    })
  } else {
    cookie = serialize(name, value, { path: '/', ...opt })
  }
  c.header('set-cookie', cookie, { append: true })
}

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  let cookie
  if (opt?.prefix === 'secure') {
    cookie = await serializeSigned('__Secure-' + name, value, secret, {
      path: '/',
      ...opt,
      secure: true,
    })
  } else if (opt?.prefix === 'host') {
    cookie = await serializeSigned('__Host-' + name, value, secret, {
      ...opt,
      path: '/',
      secure: true,
      domain: undefined,
    })
  } else {
    cookie = await serializeSigned(name, value, secret, { path: '/', ...opt })
  }
  c.header('set-cookie', cookie, { append: true })
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}
Repository: honojs/hono
File: src/helper/cookie/index.ts (TypeScript)

Code before:
    c.header('set-cookie', cookie, { append: true })
  }
}

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  const cookies = []
  if (opt?.securePrefix) {
    cookies.push(
      await serializeSigned('__Secure-' + name, value, secret, { path: '/', secure: true, ...opt })
    )
  }

  if (opt?.hostPrefix) {
    cookies.push(
      await serializeSigned('__Host-' + name, value, secret, {
        secure: true,
        path: '/',
        domain: undefined,
        ...opt,
      })
    )
  }

  if (!opt?.securePrefix && !opt?.hostPrefix) {
    cookies.push(await serializeSigned(name, value, secret, { path: '/', ...opt }))
  }
  for (const cookie of cookies) {
    c.header('set-cookie', cookie, { append: true })
  }
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}


Review comment (security):
```suggestion
      await serializeSigned('__Host-' + name, value, secret, {
        ...opt,
        secure: true,
        path: '/',
        domain: undefined,
      })
```

Code after:

export const setSignedCookie = async (
  c: Context,
  name: string,
  value: string,
  secret: string | BufferSource,
  opt?: CookieOptions
): Promise<void> => {
  let cookie
  if (opt?.prefix === 'secure') {
    cookie = await serializeSigned('__Secure-' + name, value, secret, {
      path: '/',
      ...opt,
      secure: true,
    })
  } else if (opt?.prefix === 'host') {
    cookie = await serializeSigned('__Host-' + name, value, secret, {
      ...opt,
      path: '/',
      secure: true,
      domain: undefined,
    })
  } else {
    cookie = await serializeSigned(name, value, secret, { path: '/', ...opt })
  }
  c.header('set-cookie', cookie, { append: true })
}

export const deleteCookie = (c: Context, name: string, opt?: CookieOptions): void => {
  setCookie(c, name, '', { ...opt, maxAge: 0 })
}
Repository: honojs/hono
File: src/utils/mime.ts (TypeScript)

Code before:
  mp3: 'audio/mpeg',
  mp4: 'video/mp4',
  mpeg: 'video/mpeg',
  oga: 'audio/ogg',
  ogv: 'video/ogg',
  ogx: 'application/ogg',
  opus: 'audio/opus',
  otf: 'font/otf',
  pdf: 'application/pdf',
  png: 'image/png',
  rtf: 'application/rtf',
  svg: 'image/svg+xml',
  tif: 'image/tiff',
  tiff: 'image/tiff',
  ts: 'video/mp2t',
  ttf: 'font/ttf',
  txt: 'text/plain',
  wasm: 'application/wasm',
  webm: 'video/webm',
  weba: 'audio/webm',
  webp: 'image/webp',
  woff: 'font/woff',
  woff2: 'font/woff2',
  xhtml: 'application/xhtml+xml',
  xml: 'application/xml',
  yaml: 'application/yaml',
  zip: 'application/zip',
  '3gp': 'video/3gpp',
  '3g2': 'video/3gpp2',
  gltf: 'model/gltf+json',
  glb: 'model/gltf-binary',
}


Review comment (suggestion):
Sorry, we have to reconsider it.

if it has another key for YAML mime like `yml`:

```ts
yml: 'application/yaml'
```

If more than one key exists for `application/yaml`, it is not known which extension is used. So, it may be difficult to specify the extension used in SSG by mime type alone.


Code after:
  mp3: 'audio/mpeg',
  mp4: 'video/mp4',
  mpeg: 'video/mpeg',
  oga: 'audio/ogg',
  ogv: 'video/ogg',
  ogx: 'application/ogg',
  opus: 'audio/opus',
  otf: 'font/otf',
  pdf: 'application/pdf',
  png: 'image/png',
  rtf: 'application/rtf',
  svg: 'image/svg+xml',
  tif: 'image/tiff',
  tiff: 'image/tiff',
  ts: 'video/mp2t',
  ttf: 'font/ttf',
  txt: 'text/plain',
  wasm: 'application/wasm',
  webm: 'video/webm',
  weba: 'audio/webm',
  webp: 'image/webp',
  woff: 'font/woff',
  woff2: 'font/woff2',
  xhtml: 'application/xhtml+xml',
  xml: 'application/xml',
  zip: 'application/zip',
  '3gp': 'video/3gpp',
  '3g2': 'video/3gpp2',
  gltf: 'model/gltf+json',
  glb: 'model/gltf-binary',
}
Repository: honojs/hono
File: src/middleware/trailing-slash/index.test.ts (TypeScript)

Code before:
  it('Trim', async () => {
    app = new Hono({ strict: true })

    app.use('*', trimTrailingSlash())

    app.get('/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/endpoint/without/trailing/slash', async (c) => {
      return c.text('ok')
    })

    let resp: Response

    resp = await app.request('/')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(200)

    resp = await app.request('/the/example/endpoint/without/trailing/slash/')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(301)

    resp = await app.request('/the/example/endpoint/without/trailing/slash/?exampleParam=1')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(301)

    resp = await app.request('/the/example/endpoint/without/trailing/slash')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(200)
  })

  it('Append', async () => {
    app = new Hono({ strict: true })

    app.use('*', appendTrailingSlash())

    app.get('/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/endpoint/with/trailing/slash/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/simulate/a.file', async (c) => {
      return c.text('ok')
    })

    let resp: Response

    resp = await app.request('/')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(200)

Review comment (suggestion):
Can you add the test to check the `location` header value?

And, you can write the same tests for all `301` responses.

Code after:
  it('Trim', async () => {
    app = new Hono({ strict: true })

    app.use('*', trimTrailingSlash())

    app.get('/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/endpoint/without/trailing/slash', async (c) => {
      return c.text('ok')
    })

    let resp: Response, loc: URL

    resp = await app.request('/')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(200)

    resp = await app.request('/the/example/endpoint/without/trailing/slash')
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(200)

    resp = await app.request('/the/example/endpoint/without/trailing/slash/')
    loc = new URL(resp.headers.get('location')!)
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(301)
    expect(loc.pathname).toBe('/the/example/endpoint/without/trailing/slash')

    resp = await app.request('/the/example/endpoint/without/trailing/slash/?exampleParam=1')
    loc = new URL(resp.headers.get('location')!)
    expect(resp).not.toBeNull()
    expect(resp.status).toBe(301)
    expect(loc.pathname).toBe('/the/example/endpoint/without/trailing/slash')
    expect(loc.searchParams.get('exampleParam')).toBe('1')
  })

  it('Append', async () => {
    app = new Hono({ strict: true })

    app.use('*', appendTrailingSlash())

    app.get('/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/endpoint/with/trailing/slash/', async (c) => {
      return c.text('ok')
    })
    app.get('/the/example/simulate/a.file', async (c) => {
      return c.text('ok')
    })
Repository: honojs/hono
File: src/middleware/cache/index.ts (TypeScript)

Code before:
        c.res.headers
          .get('Cache-Control')
          ?.split(',')
          .map((d) => d.trim().split('=', 1)[0]) ?? []
      for (const directive of cacheControlDirectives) {
        let [name, value] = directive.trim().split('=', 2)
        name = name.toLowerCase()
        if (!existingDirectives.includes(name)) {
          c.header('Cache-Control', `${name}${value ? `=${value}` : ''}`, { append: true })
        }
      }
    }

    if (varyDirectives) {
      const existingDirectives =
        c.res.headers
          .get('Vary')
          ?.split(',')
          .map((d) => d.trim()) ?? []
      const vary = Array.from(
        new Set(
          [...existingDirectives, ...varyDirectives].map((directive) =>
            directive.toLocaleLowerCase()
          )
        )
      ).sort((a, b) => a.localeCompare(b))

      if (vary.includes('*')) {
        c.header('Vary', '*')
      } else {
        c.header('Vary', vary.join(', '))
      }
    }
  }

  return async function cache(c, next) {
    const key = c.req.url
    const cache = await caches.open(options.cacheName)
    const response = await cache.match(key)
    if (response) {
      return new Response(response.body, response)
    }

    await next()
    if (!c.res.ok) {
      return
    }
    addHeader(c)
    const res = c.res.clone()
    if (options.wait) {
      await cache.put(key, res)

Review comment (performance):
I sorted the `Vary` to be given to the header, but I am willing to remove it if it affects performance.

Code after:
    if (cacheControlDirectives) {
      const existingDirectives =
        c.res.headers
          .get('Cache-Control')
          ?.split(',')
          .map((d) => d.trim().split('=', 1)[0]) ?? []
      for (const directive of cacheControlDirectives) {
        let [name, value] = directive.trim().split('=', 2)
        name = name.toLowerCase()
        if (!existingDirectives.includes(name)) {
          c.header('Cache-Control', `${name}${value ? `=${value}` : ''}`, { append: true })
        }
      }
    }

    if (varyDirectives) {
      const existingDirectives =
        c.res.headers
          .get('Vary')
          ?.split(',')
          .map((d) => d.trim()) ?? []

      const vary = Array.from(
        new Set(
          [...existingDirectives, ...varyDirectives].map((directive) => directive.toLowerCase())
        )
      ).sort()

      if (vary.includes('*')) {
        c.header('Vary', '*')
      } else {
        c.header('Vary', vary.join(', '))
      }
    }
  }

  return async function cache(c, next) {
    const key = c.req.url
    const cache = await caches.open(options.cacheName)
    const response = await cache.match(key)
    if (response) {
      return new Response(response.body, response)
    }

    await next()
    if (!c.res.ok) {
      return
    }
    addHeader(c)
    const res = c.res.clone()
    if (options.wait) {
Repository: honojs/hono
File: src/middleware/timeout/index.ts (TypeScript)

Code before:
  const pattern = /(\d+)(ms|s|m|h)/g
  let totalMilliseconds = 0

  let match: RegExpExecArray | null
  while ((match = pattern.exec(duration)) !== null) {
    const value = parseInt(match[1], 10)
    const unit = match[2]

    if (!units[unit]) {
      throw new Error(`Unsupported time unit: ${unit}`)
    }
    totalMilliseconds += value * units[unit]
  }

  return totalMilliseconds
}

export const timeout = (
  duration: number | string,
  options: TimeoutOptions = {}
): MiddlewareHandler => {
  const errorMessage = options.message ?? DEFAULT_ERROR_MESSAGE
  const errorCode = options.code ?? DEFAULT_ERROR_CODE
  const ms = typeof duration === 'string' ? parseDuration(duration) : duration

  return async (context, next) => {
    let timer: number | undefined

    const timeoutPromise = new Promise<void>((_, reject) => {
      timer = setTimeout(() => {
        reject(new HTTPException(errorCode, { message: errorMessage }))
      }, ms) as unknown as number
    })

    try {
      await Promise.race([next(), timeoutPromise])
    } finally {
      if (timer !== undefined) {
        clearTimeout(timer)
      }
    }
  }
}


Review comment (bug):
To make it easier for users to debug, it would be better to return a named function like other middleware.

https://github.com/honojs/hono/blob/e9e3c8ab65039a8bd2d9f3d961ff8d1fa7a6bdfe/src/middleware/body-limit/index.ts#L50

Code after:
 * ```ts
 * const app = new Hono()
 *
 * app.use(
 *   '/long-request',
 *   timeout(5000) // Set timeout to 5 seconds
 * )
 *
 * app.get('/long-request', async (c) => {
 *   await someLongRunningFunction()
 *   return c.text('Completed within time limit')
 * })
 * ```
 */
export const timeout = (
  duration: number,
  exception: HTTPExceptionFunction | HTTPException = defaultTimeoutException
): MiddlewareHandler => {
  return async function timeout(context, next) {
    let timer: number | undefined
    const timeoutPromise = new Promise<void>((_, reject) => {
      timer = setTimeout(() => {
        reject(typeof exception === 'function' ? exception(context) : exception)
      }, duration) as unknown as number
    })

    try {
      await Promise.race([next(), timeoutPromise])
    } finally {
      if (timer !== undefined) {
        clearTimeout(timer)
      }
    }
  }
}
Repository: honojs/hono
File: src/middleware/timeout/index.ts (TypeScript)

Code before:
import type { Context } from '../../context'
import { HTTPException } from '../../http-exception'
import type { MiddlewareHandler } from '../../types'

export type HTTPExecptionFunction = (context: Context) => HTTPException

const defaultTimeoutException = new HTTPException(504, {
  message: 'Gateway Timeout',
})

export const timeout = (
  duration: number,
  exception: HTTPExecptionFunction | HTTPException = defaultTimeoutException
): MiddlewareHandler => {
  return async function timeout(context, next) {
    let timer: number | undefined
    const timeoutPromise = new Promise<void>((_, reject) => {
      timer = setTimeout(() => {
        reject(typeof exception === 'function' ? exception(context) : exception)
      }, duration) as unknown as number
    })

    try {
      await Promise.race([next(), timeoutPromise])
    } finally {
      if (timer !== undefined) {
        clearTimeout(timer)
      }
    }
  }

Review comment (suggestion):
It's a typo! Should be:

```
HTTPExceptionFunction
```

Code after:
import type { Context } from '../../context'
import { HTTPException } from '../../http-exception'
import type { MiddlewareHandler } from '../../types'

export type HTTPExceptionFunction = (context: Context) => HTTPException

const defaultTimeoutException = new HTTPException(504, {
  message: 'Gateway Timeout',
})

/**
 * Timeout middleware for Hono.
 *
 * @param {number} duration - The timeout duration in milliseconds.
 * @param {HTTPExceptionFunction | HTTPException} [exception=defaultTimeoutException] - The exception to throw when the timeout occurs. Can be a function that returns an HTTPException or an HTTPException object.
 * @returns {MiddlewareHandler} The middleware handler function.
 *
 * @example
 * ```ts
 * const app = new Hono()
 *
 * app.use(
 *   '/long-request',
 *   timeout(5000) // Set timeout to 5 seconds
 * )
 *
 * app.get('/long-request', async (c) => {
 *   await someLongRunningFunction()
 *   return c.text('Completed within time limit')
 * })
Repository: honojs/hono
File: src/utils/body.ts (TypeScript)

Code before:
}

async function parseFormData<T extends BodyData = BodyData>(
  request: HonoRequest | Request,
  options: ParseBodyOptions
): Promise<T> {
  const formData = await (request as Request).formData()

  if (formData) {
    return convertFormDataToBodyData<T>(formData, options)
  }

  return {} as T
}

function convertFormDataToBodyData<T extends BodyData = BodyData>(
  formData: FormData,
  options: ParseBodyOptions
): T {
  const form: BodyData = {}

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')

    if (!shouldParseAllValues) {
      if (options.dot && key.includes('.')) {
        setNestedValue(form, key.split('.'), value)
      } else {
        form[key] = value
      }
    } else {
      handleParsingAllValues(form, key, value, options.dot)
    }
  })

  return form as T
}

const handleParsingAllValues = (
  form: BodyData,
  key: string,
  value: FormDataEntryValue,
  dot?: boolean
): void => {
  const formKey = form[key]

  if (formKey && Array.isArray(formKey)) {
    ;(form[key] as (string | File)[]).push(value)
  } else if (formKey) {
    form[key] = [formKey, value]
  } else {

Review comment (suggestion):
Can you put this expression in a variable `shouldParseNestedValues` or something

Code after:
  const headers = request instanceof HonoRequest ? request.raw.headers : request.headers
  const contentType = headers.get('Content-Type')

  if (
    (contentType !== null && contentType.startsWith('multipart/form-data')) ||
    (contentType !== null && contentType.startsWith('application/x-www-form-urlencoded'))
  ) {
    return parseFormData<T>(request, { all, dot })
  }

  return {} as T
}

/**
 * Parses form data from a request.
 *
 * @template T - The type of the parsed body data.
 * @param {HonoRequest | Request} request - The request object containing form data.
 * @param {ParseBodyOptions} options - Options for parsing the form data.
 * @returns {Promise<T>} The parsed body data.
 */
async function parseFormData<T extends BodyData = BodyData>(
  request: HonoRequest | Request,
  options: ParseBodyOptions
): Promise<T> {
  const formData = await (request as Request).formData()

  if (formData) {
    return convertFormDataToBodyData<T>(formData, options)
  }

  return {} as T
}

/**
 * Converts form data to body data based on the provided options.
 *
 * @template T - The type of the parsed body data.
 * @param {FormData} formData - The form data to convert.
 * @param {ParseBodyOptions} options - Options for parsing the form data.
 * @returns {T} The converted body data.
 */
function convertFormDataToBodyData<T extends BodyData = BodyData>(
  formData: FormData,
  options: ParseBodyOptions
): T {
  const form: BodyData = Object.create(null)

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')