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

Trigger a 404 from within view #1249

Closed
qwales1 opened this issue Mar 24, 2015 · 13 comments
Closed

Trigger a 404 from within view #1249

qwales1 opened this issue Mar 24, 2015 · 13 comments
Labels

Comments

@qwales1
Copy link

qwales1 commented Mar 24, 2015

Is there a way to pass an error out of a view to be handled by the default handlers in Keystone? What I was hoping to do was pass along a 404 if a query returned no results. Passing an error to next() doesn't stop the view from rendering.

var keystone = require('keystone');

exports = module.exports = function(req, res) {

var view = new keystone.View(req, res),
locals = res.locals;

// locals.section is used to set the currently selected
// item in the header navigation.
locals.section = 'serviceareas';

view.on('init', function(next){

    var q =  keystone.list('ServiceArea').model.find().sort('sortOrder');
    q.exec(function(err, result){
        if(err) next(err);
        if(result.length === 0){
            var notfound = new Error();
            notfound.status = 404;
            next(notfound); // any error I pass to next() doesn't stop the render call
        } else {
            locals.serviceareas = result;
            next();
        }

    });
})

// Render the view
view.render('index');

};
@morenoh149
Copy link
Contributor

try instead of passing the error, throwing it right then and there. I think that would produce a 500 error btw.

@qwales1
Copy link
Author

qwales1 commented Mar 25, 2015

Yes throwing there causes an uncaught exception. I could use the response object directly, but then would have to implement custom 404's for each route. I was hoping to just pass it along to the default handler.

@albertogasparin
Copy link
Contributor

I think this is what you are looking for:

if (result.length === 0) {
  return res.status(404).send(keystone.wrapHTMLError('Sorry, no page could be found at this address (404)'));
}

keystone.wrapHTMLError is a method that generates a string of the default error page.
In addition, you can pass any number to status() and any string as send argument. I prefer a "Not found" string and then let nginx do the error page rendering.

On a final note, the res object is quite powerful. You can also handle dynamic redirects like:

if (result.length === 0) {
  return res.status(301).redirect('/');
}

@qwales1
Copy link
Author

qwales1 commented Apr 4, 2015

@albertogasparin Thanks for pointing out keystone.wrapHTMLError . I did not know about that and that is definitely what I'm looking for as the last step in the error handling. Sorry I think I could have done a better job naming this issue in hindsight. Passing a 404 error out of the view was what I was initially trying to do when I ran into this. The real issue for me is that View doesn't have the ability to pass errors back out to the middleware chain. I think maybe it could be useful beyond just handling 404s if it did. For instance, if someone needed to change the error handling for the whole site, they wouldn't have to think about the specific view code that was written.

@jeffreypriebe
Copy link
Contributor

+1 - being able to pass errors out of the view back up for appropriate handling would be useful.

@jacargentina
Copy link
Contributor

Any advance on this? I've started writing my first keystone app by following the tutorial, which initializes a res.notfound function. See http://keystonejs.com/docs/getting-started/#routesviews-middleware

So, on a given "view route handler", when i'm accessing the database to lookup a given slug for a doc, and it's not found, should i call that res.notfound function right ?

@qwales1
Copy link
Author

qwales1 commented Aug 18, 2015

@jacargentina did you try that? I think render can still get called even though the response was already sent. Which makes me realize what I said before about using the response object directly might not be an option. @JedWatson any thoughts on this?

@mxstbr mxstbr added the bug label Apr 29, 2016
@mxstbr
Copy link
Collaborator

mxstbr commented Apr 29, 2016

Sorry for the late response on this.

This should definitely be possible, if it isn't that's a bug. If one of you could try it with the current master version and then come back to me and tell me if this problem still exists I'd be very grateful!

@qwales1
Copy link
Author

qwales1 commented May 1, 2016

@mxstbr No worries at all. Thanks for all the work you have been doing. Did you mean "its a bug" if you can't pass errors out of View back to the express? I just pulled master and that is still the case. keystone.View doesn't have access to the next callback from express and passes any error in the queues to the keystone.View.render callback. #1257 adds the ability to opt into passing the error back to express instead of keystone.View.render callback.

If you meant when I said

I think render can still get called even though the response was already sent.

That was my error. I was probably calling next after res.notfound by accident. I tried that on master and there is no problem in using the res.notfound @jacargentina pointed out from the getting started section of the website.

@mxstbr
Copy link
Collaborator

mxstbr commented May 1, 2016

Oh right, I didn't know #1257 exists! I'll poke @JedWatson to take a look.

@r3wt
Copy link

r3wt commented Jun 4, 2016

@mxstbr i think there needs to be a method for destroying the view. i get an error about the headers already being sent

@webdesign7
Copy link

How to customise 404 error page ? ( for eg. to add some links to homepage or something like that ) ?

@gautamsi
Copy link
Member

Keystone 4 is going in maintenance mode. Expect no major change. see #4913 for details.

@albertogasparin's comment is the best solution/workaround for v4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants