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

Path unmounted if error occurs #62

Open
frankshaka opened this issue Sep 11, 2017 · 2 comments
Open

Path unmounted if error occurs #62

frankshaka opened this issue Sep 11, 2017 · 2 comments

Comments

@frankshaka
Copy link
Contributor

frankshaka commented Sep 11, 2017

When using request logging or error capturing middlewares, the path printed in the log will be incorrect, if any error was thrown from within the mounted app.

Example:

// a casual request logging middleware
app.use(async (ctx, next) => {
    console.log("[>>>] %s %s", ctx.method, ctx.path)
    try {
        await next()
    } catch(err) {
        if (err.status) {
            ctx.status = err.status
        }
    } finally {
        console.log("[%s] %s %s", ctx.status, ctx.method, ctx.path)
    }
})

// mouting another app
app.use(mount('/api', async (ctx, next) => {
    return ctx.throw(403, 'Forbidden')
}))

The result would be:

[>>>] GET /api/user
[403] GET /user

instead of what is expected:

[>>>] GET /api/user
[403] GET /api/user

IMHO it's better to wrap mounts within try-finally blocks, like this diff I created.

@jonathanong
Copy link
Member

do you mind creating a failing test case as a PR?

frankshaka added a commit to frankshaka/mount that referenced this issue Oct 9, 2017
@frankshaka
Copy link
Contributor Author

I've created a PR with a failing test case for this. Running npm test will print out an AssertionError.

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

No branches or pull requests

2 participants