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

feat(middleware): Introduce Timeout Middleware #2615

Merged
merged 22 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
1 change: 1 addition & 0 deletions deno_dist/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export { jwt } from './middleware/jwt/index.ts'
export * from './middleware/logger/index.ts'
export * from './middleware/method-override/index.ts'
export * from './middleware/powered-by/index.ts'
export * from './middleware/timeout/index.ts'
export * from './middleware/timing/index.ts'
export * from './middleware/pretty-json/index.ts'
export * from './middleware/secure-headers/index.ts'
Expand Down
61 changes: 61 additions & 0 deletions deno_dist/middleware/timeout/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { HTTPException } from '../../http-exception.ts'
import type { MiddlewareHandler } from '../../types.ts'
import type { StatusCode } from '../../utils/http-status.ts'

interface TimeoutOptions {
message?: string
code?: StatusCode
}

const DEFAULT_ERROR_MESSAGE = 'Gateway Timeout'
const DEFAULT_ERROR_CODE = 504

interface DurationUnits {
[key: string]: number
}

const parseDuration = (duration: string): number => {
const units: DurationUnits = { ms: 1, s: 1000, m: 60000, h: 3600000 }
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)
}
}
}
}
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@
"import": "./dist/middleware/jwt/index.js",
"require": "./dist/cjs/middleware/jwt/index.js"
},
"./timeout": {
"types": "./dist/types/middleware/timeout/index.d.ts",
"import": "./dist/middleware/timeout/index.js",
"require": "./dist/cjs/middleware/timeout/index.js"
},
yusukebe marked this conversation as resolved.
Show resolved Hide resolved
"./timing": {
"types": "./dist/types/middleware/timing/index.d.ts",
"import": "./dist/middleware/timing/index.js",
Expand Down
1 change: 1 addition & 0 deletions src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export { jwt } from './middleware/jwt'
export * from './middleware/logger'
export * from './middleware/method-override'
export * from './middleware/powered-by'
export * from './middleware/timeout'
export * from './middleware/timing'
export * from './middleware/pretty-json'
export * from './middleware/secure-headers'
Expand Down
70 changes: 70 additions & 0 deletions src/middleware/timeout/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { Hono } from '../../hono'
import { timeout } from '.'

describe('Timeout API', () => {
const app = new Hono()
app.use('/slow-endpoint', timeout(1000))
app.use(
'/slow-endpoint/custom',
timeout('1s100ms', {
message: 'Request timeout. Please try again later.',
code: 408,
})
)
app.use(
'/slow-endpoint/error',
timeout('1s2000ms', {
message: 'Request timeout. Please try again later.',
code: 408,
})
)
app.use('/normal-endpoint', timeout(1000))

app.get('/slow-endpoint', async (c) => {
await new Promise((resolve) => setTimeout(resolve, 1100))
return c.text('This should not show up')
})

app.get('/slow-endpoint/custom', async (c) => {
await new Promise((resolve) => setTimeout(resolve, 1100))
return c.text('This should not show up')
})

app.get('/slow-endpoint/error', async (c) => {
await new Promise((resolve) => setTimeout(resolve, 1100))
return c.text('This should not show up')
})

app.get('/normal-endpoint', async (c) => {
await new Promise((resolve) => setTimeout(resolve, 900))
return c.text('This should not show up')
})

it('Should contain total duration', async () => {
const res = await app.request('http://localhost/slow-endpoint')
expect(res).not.toBeNull()
expect(res.status).toBe(504)
expect(await res.text()).toContain('Gateway Timeout')
})

it('Should apply custom error message and status code', async () => {
const res = await app.request('http://localhost/slow-endpoint/custom')
expect(res).not.toBeNull()
expect(res.status).toBe(408)
expect(await res.text()).toContain('Request timeout. Please try again later.')
})

it('Error timeout', async () => {
const res = await app.request('http://localhost/slow-endpoint/error')
expect(res).not.toBeNull()
expect(res.status).toBe(200) // Confirming the endpoint does not trigger a timeout as expected
expect(await res.text()).toContain('This should not show up')
})

it('No Timeout', async () => {
const res = await app.request('http://localhost/normal-endpoint')
expect(res).not.toBeNull()
expect(res.status).toBe(200)
expect(await res.text()).toContain('This should not show up')
})
})
61 changes: 61 additions & 0 deletions src/middleware/timeout/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { HTTPException } from '../../http-exception'
import type { MiddlewareHandler } from '../../types'
import type { StatusCode } from '../../utils/http-status'

interface TimeoutOptions {
message?: string
code?: StatusCode
}

const DEFAULT_ERROR_MESSAGE = 'Gateway Timeout'
const DEFAULT_ERROR_CODE = 504

interface DurationUnits {
[key: string]: number
}

const parseDuration = (duration: string): number => {
const units: DurationUnits = { ms: 1, s: 1000, m: 60000, h: 3600000 }
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 = {}
Copy link
Member

Choose a reason for hiding this comment

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

How about taking exception as the second arg:

export const timeout = (
  duration: number | string,
  exception?: HTTPException | ((c: Context) => HTTPException)
)

How to use it (callback pattern):

app.get(
  '/',
  timeout(1000, (c) => {
    return new HTTPException(408, {
      message: `Timeout occurred at ${c.req.path}`,
    })
  }),
  async (c) => {
    // ...
  }
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice. I fixed it, can you see if the type definitions and other perceptions you think match?

): 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) => {
Copy link
Member

@usualoma usualoma May 7, 2024

Choose a reason for hiding this comment

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

To make it easier for users to debug, it would be better to return a named function like other middleware.

return async function bodyLimit(c, next) {

Copy link
Contributor Author

@watany-dev watany-dev May 7, 2024

Choose a reason for hiding this comment

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

thx! I fixed 758a334.

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)
}
}
}
}