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

Abort sequence issue #686

Open
nomocas opened this issue Dec 10, 2024 · 1 comment · May be fixed by #685
Open

Abort sequence issue #686

nomocas opened this issue Dec 10, 2024 · 1 comment · May be fixed by #685

Comments

@nomocas
Copy link

nomocas commented Dec 10, 2024

Hello All,

Env:
OSX: Darwin Kernel Version 23.6.0 arm64 (apple silicon)
Deno: v2.1.3
Oak: v17.3.1

There is an issue with the way the abort sequence is managed. If we take the example from the github docs, the promise returned by the app.listen is never resolved in any case, and when the abort is triggered right before the await listenPromise (as in the example), the close event is never dispatched (we see in the logs results that the abort event is emitted before the server.listen promise is resolved).

import { Application } from '@oak/oak/application'

const app = new Application()

const controller = new AbortController()
const { signal } = controller

// Dummy middleware or it throws
app.use((_, next) => {
	console.log('Middleware 1')
	return next()
})

const listenPromise = app.listen({ port: 8000, signal })

// dispatched after the server.listen promise resolves
app.addEventListener('listen', () => {
	console.log('Server is listening')
})

// Never dispatched! (it should have been dispatched after the abort controller is aborted but nothing happened)
app.addEventListener('close', () => {
	console.log('Server closed (event)')
})

controller.signal.addEventListener('abort', () => {
	console.log('Abort!')
})

controller.abort()

// On abort: Listen should stop listening for requests and the promise should resolve...
await listenPromise

// Never reached!
console.log('Server closed (promise resolved)')

Logs result:

Abort!
Server is listening

So nothing happens and the process is stuck.

If we add a setTimeout around the controller.abort():

import { Application } from '@oak/oak/application'

const app = new Application()

const controller = new AbortController()
const { signal } = controller

// Dummy middleware or it throws
app.use((_, next) => {
	console.log('Middleware 1')
	return next()
})

const listenPromise = app.listen({ port: 8000, signal })

// dispatched after the server.listen promise resolves
app.addEventListener('listen', () => {
	console.log('Server is listening')
})

// dispatched after the abort controller is aborted
app.addEventListener('close', () => {
	console.log('Server closed (event)')
})

// never dispatched (so no underlying error)
app.addEventListener('error', (error) => {
	console.error('Server error (event)', error)
})

controller.signal.addEventListener('abort', () => {
	console.log('Abort!')
})

setTimeout(() => {
	controller.abort()
}, 1000)

// On abort: Listen should stop listening for requests and the promise should resolve...
await listenPromise

// Never reached!
console.log('Server closed (promise resolved)')

Logs results:

Server is listening
Abort!
Server closed (event)
error: Top-level await promise never resolved
await listenPromise
^

Again, the listenPromise is never resolved and it throws an error.

When I check the @oak/oak/application/Application.listen code, it seems to be stuck in the loop (line 852) or the subsequent await:

      for await (const request of server) {
        this.#handleRequest(request, secure, state);
      }
      await Promise.all(state.handling);

Any clue ?

Thank you.

@nomocas
Copy link
Author

nomocas commented Dec 10, 2024

Oh, I see that there is already a PR for it.

#685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant