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

[fix] Very odd behavior with router.param() handlers #194

Open
3 tasks done
joekrill opened this issue Nov 8, 2024 · 0 comments
Open
3 tasks done

[fix] Very odd behavior with router.param() handlers #194

joekrill opened this issue Nov 8, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@joekrill
Copy link

joekrill commented Nov 8, 2024

Describe the bug

Node.js version: 22.8.0

OS version:

Description: router.param() handlers are not called consistently, may be called multiples times, or not called at all, depending on the order they were created (and in particular, the order in relation to other verb handlers).

Actual behavior

When param() handlers are defined, whether they get called, how many times they get called, and in what order depends entirely on when the related verb was registered. This mostly happens when using multiple param() handlers for the same parameter. But there are also issues with just a single param() handler being called multiple times in some cases (see the last test in the example repo for a case of this happening).

For example, given the following router:

const router = new Router();
router
  .param('id', (id, ctx, next) => {
    console.log("param handler 1");
    return next();
  })
  .param('id', (id, ctx, next) => {
    console.log("param handler 2");
    return next();
  })
  .param('id', (id, ctx, next) => {
    console.log("param handler 3");
    return next();
  })
  .get('/:id', (ctx) => {
    console.log("get handler");
    ctx.status = 200;
  });

a GET /:id request will ONLY call the last param handler. The first 2 handlers are ignored completely.

Then there's this scenario where, if you have multiple matching verb handlers, some of the param handlers can get called multiple times:

router
  .param('id', (id, ctx, next) => {
    console.log("param handler 1");
    return next();
  })
  .get('/:id', (ctx, next) => {
    console.log("get handler 1");
    return next();
  })
  .param('id', (id, ctx, next) => {
    console.log("param handler 2");
    return next();
  })
  .param('id', (id, ctx, next) => {
    console.log("param handler 3");
    return next();
  })
  .get('/:id', (ctx) => {
    console.log("get handler 2");
    ctx.status = 200;
  });

In this case a GET /:id request will result in:

  • param handler 1
  • param handler 2
  • param handler 3
  • get handler 1
  • param handler 2
  • param handler 3
  • get handler 2

so param handlers 2 and 3 get called twice.

Expected behavior

All param() handlers should be called once, in order, for any verb that uses that parameter, regardless of what order they were registered in. Or

-- or --

The documentation should be updated to make it clear that only a single param() handler per param name is supported. But there is still a bug where a param() handler can get called more than once, like here:

 router
  .use('/:id', (ctx, next) => {
    console.log(use1');
    next();
  })
  .param('id', (id, ctx, next) => {
    // This gets called twice for a `GET /:id` request
    console.log('param1');
    next();
  })
  .get('/:id', (ctx) => {
    console.log('get1');
    ctx.status = 200;
  });

Code to reproduce

Tests added to this branch showing the bug:

https://github.com/joekrill/router/tree/params-bug

npm run test:params

Checklist

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
  • I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.
@joekrill joekrill added the bug Something isn't working label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant