Possibility to define a default route with Flatiron #40

Open
mikl opened this Issue Mar 13, 2012 · 12 comments

Comments

Projects
None yet
7 participants
Contributor

mikl commented Mar 13, 2012

I have not been able to find a better way to make a default route than this:

app.router.get(/((\w|.)*)/, function () {
  return nice404page();
}

And by default, Flatiron’s 404 page just has the word “undefined” on it. Not really a compelling user experience ;)

Contributor

mikl commented Mar 13, 2012

@dscape that seems like a rather nasty hack if you ask me. Default route is not an error condition, and it is something that any decent webapp should have, so I think Flatiron should have a better way to define one.

Owner

dscape commented Mar 14, 2012

@miki totally agree.

director should support a catch all route:

app.router.get(function () {
  return nice404page();
});

Alternative syntax which is less node like would be .on("*", function(...

Honestly I think regular expressions dont belong in routers, but I guess that's a matter of preference.

flatiron should have a notfound function, and some sensible 404 page.

Pull requests are always welcome :)

Owner

dscape commented Mar 14, 2012

@miki in my limited knowledge onError is not triggered by an error, but by a 404 so my solution is a valid one

Contributor

pksunkara commented Mar 16, 2012

@mikl onError is triggered only when a route is not found. So you can use it for fixing 404 errors.

@dscape @mikl Can we close this issue?

Contributor

mikl commented Mar 16, 2012

@pksunkara well, I think the Flatiron devs should consider having a more obvious shortcut to creating a default/fallback route, or at the very least, document how to do it with the current API (currently, you'll have to read the source for both Director and Flatiron (and possibly Union as well) to figure it out.

Contributor

pksunkara commented Mar 16, 2012

@mikl Sure, definitely.

@pksunkara pksunkara closed this Mar 16, 2012

@dscape dscape reopened this Mar 16, 2012

Owner

dscape commented Mar 16, 2012

I don't think this issue is resolved.

This is an obvious issue, having a work around is great to help the user but flatiron needs a better way to do this

tillre commented May 11, 2012

Director has a notfound option which is called if no route is found. But because Router.dispatch returns false in the case that no route was found and notfound is set/called, the flatiron.plugin.http in app.listen still calls res.emit('next') and unions sends an notFound Error down the stream.

If noError is set, the emit will not happen. I think Router.dispatch should return true in the case that a function is set to notfound. Then notfound can handle the case and either call the error handler it gets passed as arg or write a 404 page to the stream. Then its sufficient to set app.router.notfound = function(callback)... and handle all non matched routes there.

Has there been any official resolution to this? Are we still looking for contributions or is the onError method the way to go? Also, it seems Skoni's suggestion to change the return value of a 404 when notfound is set a function makes sense.

I also came across the Director notfound option but, from existing documentation wasn't quite able to figure out how to make this work within flatiron as the router object isn't explicitly created (where you can pass in this option) but just seems cast into existence from the perspective of the dev using flatiron. I am going to dig around in the flatiron source code to get a better understanding of how it wraps Director, I think this is an area that is a bit baffling to anyone new to using flatiron / director (as I am ).

To handle the 404 condition you could attach a function to app.router.notfound. I believe this is what @skoni mentioned

app.router.notfound = function(callback) {
    var handled = fourOhFour(this.res);
    if(!handled) { //call the default not found handler
        callback(null, this.req, this.res);
    }
};
function fourOhFour(res) {
    res.writeHead(404);
    res.json({user: 'obiwan', droids:'not found'});
    return true; //return false for some conditional reason
};

or just as easily handle the 404 and ignore the callback

app.router.notfound = function() {
    this.res.json({error:404});
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment