koajs / joi-router

Configurable, input and output validated routing for koa
MIT License
450 stars 96 forks source link

Error: next() called multiple times #97

Closed kpopovic closed 3 years ago

kpopovic commented 4 years ago

Hello,

when defined multiple http status codes in output error is thrown.

  1. when only "400-599" is defined then ot works
  2. when "400-599" and 200 are defined the error is thrown.

It seams that it has issues wtih multiple status codes.

const joiSchema = {
  body: {
    email: Joi.string()
      .lowercase()
      .email()
      .max(45)
      .required(),
    password: Joi.string()
      .max(10)
      .required()
  },
  output: {
    200: {
      body: {
        jwtToken: Joi.string()
          .alphanum()
          .min(3)
          .max(200)
          .required()
      }
    },
    "400-599": {
      body: {
        error: Joi.object()
      }
    }
  },
  type: "json",
  continueOnError: true
};

Error: next() called multiple times at dispatch (/home/kresimir/Development/zimmerfrei/zimmerfrei-http-server/node_modules/koa-router/node_modules/koa-compose/index.js:38:45) at next (/home/kresimir/Development/zimmerfrei/zimmerfrei-http-server/node_modules/koa-router/node_modules/koa-compose/index.js:45:18) at errorHandler (/home/kresimir/Development/zimmerfrei/zimmerfrei-http-server/node_modules/koa-joi-router/joi-router.js:285:22) at process._tickCallback (internal/process/next_tick.js:68:7)

aheckmann commented 4 years ago

Thank you for the report. I'm unable to reproduce this one. When you get a chance, could you provide a script I ran run which reproduces the error?

forcyan commented 4 years ago

@aheckmann Did you capture validation error and handler-generated error with same handler which will check continueOnError and call next() ?

aheckmann commented 4 years ago

I've done everything I can to reproduce without success. Please post a script I can run to reproduce the error.

-- Aaron

On Fri, Dec 13, 2019, 7:00 AM forcyan notifications@github.com wrote:

@aheckmann https://github.com/aheckmann Did you capture validation error and handler-generated error with same handler which will check continueOnError and call next() ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/koajs/joi-router/issues/97?email_source=notifications&email_token=AABIXMTKAHW4XNUUVMBGDRDQYOPRRA5CNFSM4JLEG6E2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG2GZLA#issuecomment-565472428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIXMQGQYLFZ74A7DWIYLLQYOPRRANCNFSM4JLEG6EQ .

forcyan commented 4 years ago

@aheckmann Seems only happen on validated body? I'm not sure.

const Koa = require('koa')
const Router = require("koa-joi-router")
const Joi = Router.Joi

const app = new Koa()
const router = new Router()

router.get("/", async (ctx, next) => {
    ctx.body = `
<form action="/test" method="post">
    <input type="text" name="q" value="test" />
    <input type="submit" value="submit" />
</form>`
})

router.post("/test", {
    validate: {
        type: "form",
        body: { q: Joi.string().required(), },
        continueOnError: true
    },
}, async (ctx, next) => {
    throw new Error("test error")
})

app.use(router.middleware())

app.listen(80, () => console.log("App running."))
aheckmann commented 4 years ago

Thank you. I'll take another look today.

-- Aaron

On Fri, Dec 13, 2019, 8:06 AM forcyan notifications@github.com wrote:

@aheckmann https://github.com/aheckmann Seems only happen on validated body? I'm not sure.

const Koa = require('koa')const Router = require("koa-joi-router")const Joi = Router.Joi const app = new Koa()const router = new Router() router.get("/", async (ctx, next) => { ctx.body = <form action="/test" method="post"> <input type="text" name="q" value="test" /> <input type="submit" value="submit" /></form> }) router.post("/test", { validate: { type: "form", body: { q: Joi.string().required(), }, continueOnError: true }, }, async (ctx, next) => { throw new Error("test error") }) app.use(router.middleware()) app.listen(80, () => console.log("App running."))

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/koajs/joi-router/issues/97?email_source=notifications&email_token=AABIXMUDSOLQJJDM44WBFPTQYOXJ7A5CNFSM4JLEG6E2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG2NMAQ#issuecomment-565499394, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIXMV7TZELV6RKZFB22E3QYOXJ7ANCNFSM4JLEG6EQ .

kpopovic commented 4 years ago

Hello, I gave up on "koa-joi-router", don't use it any more. Deleted the project sample with this error. Sorry.

rolivares commented 4 years ago

I my case same error is appearing when continueOnError: true but the error is not informed by Joi for example an undefined var on handler body

handler : async () => {
   // const a = 1
  await otherTier.myFunction(a)
}

So, I've simple add the validation (for testing) if the error is a joi error in order to continue whenever joi use continueOnError === true, otherwise the error must be thrown

if (!(err.isJoi && spec.validate.continueOnError)) return ctx.throw(err);

if (err.isJoi && spec.validate.continueOnError) {
        return await next();
      } else {
        return ctx.throw(err);
      }
denysaw commented 4 years ago

Confirming. Sadly, the issue still exists in the latest version. Steps to reproduce:

validate: {
    ...
    continueOnError: true
},
handler: async (ctx) => {
    if (ctx.invalid) {
        const e = ctx.invalid.body.details[0];
        ctx.throw(406, JSON.stringify({ errors: { [e.context.key]: e.message } }));
    }
}

Call that endpoint and you'll get:

Error: next() called multiple times
      at dispatch (PROJECT_PATH\node_modules\koa-compose\index.js:38:45)
      at next (PROJECT_PATH\node_modules\koa-compose\index.js:45:18)
      at errorHandler (PROJECT_PATH\node_modules\koa-joi-router\joi-router.js:285:22)
mdpx commented 4 years ago

The problem is that when you throw an error (either in code explicitly or by runtime) in the route handler, the errorHandler catch the error and calls next() which breaks koa's compose function. Although "an undefined var on handler body" is not a good example, @rolivares 's workaround works for custom errors, which is my case, but won't work if I throw the Joi error from ctx.invalid as-is, which maybe nobody would really do that, it still should be taken into consideration IMO.