Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

response code 500 on accessing non-existing document? #348

Closed
FREEZX opened this issue Jan 8, 2015 · 5 comments
Closed

response code 500 on accessing non-existing document? #348

FREEZX opened this issue Jan 8, 2015 · 5 comments

Comments

@FREEZX
Copy link

FREEZX commented Jan 8, 2015

Hi,

I'm currently developing an application using mean, and something caught my attention:
https://github.com/meanjs/mean/blob/master/app/controllers/articles.server.controller.js#L93

This returns 500 internal server error if the document does not exist.
In my opinion this should send back 404 Not Found, without disclosing a stack trace.

@lirantal
Copy link
Member

lirantal commented Jan 9, 2015

You're right @FREEZX so I committed a PR for that.
Notice though, that there's no specific route calling for getting an article by id, but rather it's a route parameter resolution.

@ilanbiala
Copy link
Member

@lirantal isn't the specific route this one? That just calls articleById internally and sends the result.

@lirantal
Copy link
Member

@ilanbiala that's the route but it uses the :articleId parameter which is resolved using express's app.param() method.

@ilanbiala
Copy link
Member

@lirantal yep, that's what I said. So what's the consensus, should we at least send a 400 if the id is not a valid ObjectId? I think we should start with that and work forward.

lirantal added a commit that referenced this issue Jan 14, 2015
fixing issue #348 - instead of returning a server error 500 on article loading which isnt found we'll throw a 404 with json message
@lirantal
Copy link
Member

merged the relevant PR so I'm closing this.

@lirantal lirantal self-assigned this Jan 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants