Add shortcut for using a custom catch-all handler

[npup]

Implemented here as an option to the server instantiation.

    const server = pogo.server({
        catchAll: async (request, h) => {
            return h.response(`My custom response, ${ request.method }`)
                .code(404);
        }
    });

I started to use pogo to create a development server and quickly realized i wanted to create a catch-all handler for missing routes (akin to the wildcard method matching feature). Happy if you will consider this contribution in some way!

Best regards /p

[npup]

Ok. I just saw the typescript conversion effort in #14 - If that happens anytime soon, I can come back or update this PR for typescript. Else, feel free to merge this first :D

[sholladay]

Thanks for this! Definitely an important feature.

Regarding the public API for catch-all routes, I like how you've made it a named option, it's pretty intuitive and clear, easy to tell what it does. We will, though, need to consider how this interplays with wildcard routes in the router, which is a feature that needs to be implemented soon, regardless.

Let's say a user writes something like this:

const server = pogo.server({
    catchAll(request, h) {
        // First catch-all handler 
    }
});

server.router.all('/{path*}', (request, h) => {
    // Second catch-all handler
});

In the case above, the first handler would be "masked" by the second - it would never run. Ideally, it should throw an error instead of allowing you to configure things "incorrectly" like that, but doing so would require a different approach because right now the server's catchAll option logic does not coordinate with the router in any way, other than being a fallback.

I think the solution is going to involve making this a router option, rather than a server option. Then the router can leverage it's existing logic for checking the routing table, etc.

We don't necessarily have to go that direction in this PR, especially since support for wildcard paths is not yet implemented in the router. But that's something to think about.

[npup]

Oh I see, good points!

But even with route wildcards (which I'd love) - a request might miss all those expressions registered there, and there are still use cases for a custom server fallback, as I see it. There is no real conflict between them.

I am interested in helping with any of these efforts.

[sholladay]

But even with route wildcards (which I'd love) - a request might miss all those expressions registered there ...

I don't see how. If a user calls server.router.all('/{path*}', handler), then handler should receive all requests that didn't match more specific routes. It's a genuine catch-all. That's actually how you do it in hapi...

const server = hapi.server();
server.route({
    method : '*',
    path   : '/{path*}',
    handler
});

Maybe there's something unclear about the above pattern, which is why I like the idea of having a named option for it.

Here's what I'm thinking we should do, at least once wildcard paths are supported.

1. The server should support a router options object. It should pass this object to new Router() when it constructs its internal router.
2. The router options object should support a catchAll function (like the one in this PR).
3. When the router receives the catchAll option, it should de-sugar the option into an actual route with method: '*' and path: '/{path*}', which it should then try to insert into the routing table using the normal route adding logic.

With this approach, I think we can solve the problems I mentioned earlier. You would use it like this...

const server = pogo.server({
    router : {
        catchAll(request, h) {
            // ...
        }
    }
});

Then, because the router actually adds it to the routing table, if a user tries to do server.router.all('/{path*}', handler), it will throw a helpful error indicating that such a route already exists.

[npup]

I don't see how. If a user calls server.router.all('/{path*}', handler), then handler should receive all requests that didn't match more specific routes.

Sorry if I'm being thick, but I interpreted an entry like the above to match all requests that start with /path - not all imaginable requests. So when all wildcard routes like that have failed, I would still find it useful to be able to override the internal bang(notFound) that is the ultimate result presently.

Then, of course: A wildcard route like "/{*}", or however one would designate it, should work, right? I suppose that is what would be generated in the desugaring process, if the option to send it as a server option is made available.

Anyway - please let me know if there is anything I can do to help this project. I really like it, thanks a lot :)

[sholladay]

I interpreted an entry like the above to match all requests that start with /path

I see! No, '/{path*}' would match any and all request paths. You could use any parameter name there, it doesn't have to be path. You could use '/{animal*}' and it would still match any and all paths. The reason for giving the parameter a name at all is so that you can access its value in the handler using request.params. For example, '/users/{userId}' lets you access the user ID as request.params.userId, so if a request comes in whose path is '/users/abc', then request.params.userId would be 'abc'. For '/{animal*}', request.params.animal would actually be the same as request.path, since it starts at the root / and the wildcard matches any number of segments.

A wildcard route like "/{*}", or however one would designate it, should work, right?

Yeah, once the router supports wildcard paths this should be easy.

Right now, using the current release version of Pogo, you can already use path parameters and do something like this:

const server = pogo.server()
server.router.all('/{page}', (request, h) => {
    // ...
});

And that route will match any path with a single path segment. So it will match /home and /users. The problem is it doesn't match multi-segment paths like /users/abc and it also doesn't match the root /. Wildcards will fix both of those problems.

Anyway - please let me know if there is anything I can do to help this project. I really like it, thanks a lot :)

Thank you! This PR is useful. I think what I'm going to do is roughly...

1. Get #14 merged
2. Look into what it would take to support wildcard paths
3. Based on how much effort wildcard paths are, either implement them first and then update this PR, or merge this as a temporary solution and revisit the implementation later when wildcard paths are supported.

It will take me at least a few days to finish looking into this.

If you have time, I could certainly use help solving the TypeScript errors in #14.

[sholladay]

Small update: #14 is merged. I'm starting to look at the router and thinking about how to implement wildcard routes.

[sholladay]

Another update here. I have made a lot of progress on the new router. I pushed the router-v2 branch, which contains the changes. It includes updated TypeScript types, support for optional path parameters, support for wildcard path parameters, and official support for Deno 1.0. It's almost functional. Still need to add the finishing touches and write some tests, but getting close on this!

[sholladay]

Catch-all routes are now functional, so I opened a PR: https://github.com/sholladay/pogo/pull/29

[sholladay]

@npup the PR for the new router with catch-all routes has been merged and I wrote some new routing documentation that has an example of using a catch-all route.

We're ready to move forward with the catchAll server option now. Could you update the implementation to add a catch-all route to the router?

[sholladay]

LGTM. Just did a bit of cleanup. Let me know if anything doesn't seem right.

Thanks, @npup! 🎉