Skip to content

Conversation

@aprici7y
Copy link

@aprici7y aprici7y commented Oct 7, 2025

Detects when a router is mounted on itself (directly or indirectly) and emits a warning to help developers identify cycles that cause requests to hang indefinitely.

When a router is mounted on itself through a chain of routers, requests that traverse the cycle hang indefinitely and eventually cause a stack overflow. This is difficult to debug because the application appears to work fine until a request hits the cyclic path.

This implements cycle detection in Router.prototype.use that:

  • Detects both direct self-mounts (router.use(router)) and indirect cycles (routerA -> routerB -> routerA)
  • Uses recursive traversal with a visited set to prevent infinite loops during detection
  • Emits a process.emitWarning() with type RouterCycleWarning
  • Maintains backward compatibility - the mount still succeeds but with a warning

Manual Test

const Router = require('router')

// Direct self-mount
const loop = new Router()
loop.use('/', loop) // Emits RouterCycleWarning

// Indirect cycle
const routerA = new Router()
const routerB = new Router()
routerA.use('/b', routerB)
routerB.use('/a', routerA) // Emits RouterCycleWarning

Fixes expressjs/express#6809

Router cycles cause silent runtime hangs when a router is mounted
on itself (directly or indirectly). This change detects such cycles
and emits a warning to help developers identify the issue quickly.

The implementation:
- Detects cycles in Router.prototype.use before mounting
- Uses recursive traversal with visited set to avoid infinite loops
- Detects both direct self-mounts and indirect cycles
- Emits process warning but keeps behavior unchanged (warn-only)
- Can be turned into an error in future major version

Tests added for:
- Direct self-loop (router.use(router))
- Indirect cycle (routerA -> routerB -> routerA)
- Nested indirect cycles (routerA -> routerB -> routerC -> routerA)
- Ensuring non-cyclic mounts don't trigger warnings
- Verifying behavior remains unchanged (mount succeeds with warning)

Related to expressjs/express#6809
* @private
*/

function detectRouterCycle (parentRouter, childRouter, visited) {

Choose a reason for hiding this comment

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

You run this function each time, so why not adding a Set/Map to avoid repetitive code?

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 this pull request may close these issues.

Router cycles cause silent runtime hangs

2 participants