Skip to content

fix: preserve headers when throwing from AsyncGenerator #1679

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.

Sign up for GitHub

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

Open
raunak-rpm wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −2
Open

fix: preserve headers when throwing from AsyncGenerator #1679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 14 additions & 2 deletions src/compose.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,14 @@ export const isAsync = (v: Function | HookContainer) => {
if (isObject && v.isAsync !== undefined) return v.isAsync

const fn = isObject ? v.fn : v
if (fn.constructor.name === 'AsyncFunction') return true
// Check for both AsyncFunction and AsyncGeneratorFunction
// AsyncGeneratorFunction needs to be treated as async because generator.next()
// returns a Promise that may reject (e.g., when throwing from the generator)
if (
fn.constructor.name === 'AsyncFunction' ||
fn.constructor.name === 'AsyncGeneratorFunction'
)
return true

const literal: string = fn.toString()

Expand Down Expand Up @@ -862,7 +869,12 @@ export const composeHandler = ({

const mapResponse = (r = 'r') => {
const after = afterResponse()
const response = `${hasSet ? 'mapResponse' : 'mapCompactResponse'}(${saveResponse}${r}${hasSet ? ',c.set' : ''}${mapResponseContext})\n`
// When maybeStream is true, mapResponse may return a Promise (from handleStream)
// that can reject if the generator throws. We need to await it so the try-catch
// can properly catch the rejection and route it to error handling.
// Only add await if the function is async (maybeAsync), otherwise it would be a syntax error.
const awaitStream = maybeStream && maybeAsync ? 'await ' : ''
const response = `${awaitStream}${hasSet ? 'mapResponse' : 'mapCompactResponse'}(${saveResponse}${r}${hasSet ? ',c.set' : ''}${mapResponseContext})\n`

if (!after) return `return ${response}`

Expand Down
45 changes: 45 additions & 0 deletions test/response/stream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,4 +560,49 @@ describe('Stream', () => {
expect(result).toEqual(['Elysia', 'Eden'].map((x) => `data: ${x}\n\n`))
expect(response.headers.get('content-type')).toBe('text/event-stream')
})

// Issue #1677: Throwing from AsyncGenerator should preserve headers
it('should preserve headers when throwing from async generator', async () => {
const { status: statusFn } = await import('../../src')

const app = new Elysia().get('/', async function* ({ set }) {
set.headers['access-control-allow-origin'] = '*'
set.headers['x-custom-header'] = 'test-value'
// Throw before yielding - this is the bug scenario from #1677
if (true) throw statusFn(500)
yield 'unreachable'
})

const response = await app.handle(req('/'))

expect(response.status).toBe(500)
expect(response.headers.get('access-control-allow-origin')).toBe('*')
expect(response.headers.get('x-custom-header')).toBe('test-value')
})

// Issue #1677: onError hook should be called when throwing from generator
it('should call onError hook when throwing from async generator', async () => {
const { status: statusFn } = await import('../../src')
let onErrorCalled = false
let errorCode: string | number | undefined

const app = new Elysia()
.onError(({ code }) => {
onErrorCalled = true
errorCode = code
})
.get('/', async function* ({ set }) {
set.headers['x-custom-header'] = 'test-value'
// Throw before yielding - this is the bug scenario from #1677
if (true) throw statusFn(500)
yield 'unreachable'
})

const response = await app.handle(req('/'))

expect(response.status).toBe(500)
expect(onErrorCalled).toBe(true)
expect(errorCode).toBe(500)
expect(response.headers.get('x-custom-header')).toBe('test-value')
})
})
Loading