A better way of handling this?

JavaScript
1

Hello,
This question is in regard to the Information Security and Quality Assurance project “Personal Library”. Can someone with better, wiser eyes offer an opinion?

Story 8 is this: “If I try to request a book that doesn’t exist I will get a ‘no book exists’ message.”

My code for this is the following. My code passes the “FCC backend tester” for this particular story but the same code times out when I test via Postman. Can anyone see why or suggest an alternative way that I might go about fulfilling this user story?

  app.route('/api/books/:id')
    .get(function (req, res){
      var bookid = req.params.id;
      //json res format: {"_id": bookid, "title": book_title, "comments": [comment,comment,...]}
    
      Book
        .findOne({_id: bookid})
        .then(book => {
          if(!book){
            return res.send("no book exists");
          }else {
            return res.json(book);
          }
        });
    
    })
2

Hard to know from just this snippet of code.

The only thing I can see from this is maybe .findOne({_id: bookid}) wants a number for bookid but it might be a string you are giving it?

3

Thanks for the response. I see how I’m unclear! I’ll look at my code again and formulate a clearer question if I can’t figure things out on my own.

This coding stuff is a constant battle against laziness!

4

Did you try it the usual way, just using a callback inside your findOne() query?

For example,

Book.findOne({ _id: bookid }, function (err, book) {
     // does book exist? ... do the stuff. If not, do the other stuff
});

https://mongoosejs.com/docs/api.html#model_Model.findOne

Also,… from the docs,

If you’re querying by _id , use findById() instead.

1 Like
5

Hey, thanks for taking the time. I originally tried the callback route, got frustrated because of amnesia and then tried the promise/.then route–which works now btw. I am not satisfied with my solution but it passes so maybe I’ll just pass to the next challenge for now.

My original callback code was as follows but it became clear that I had a misstep somewhere so just scraped it. Is there something that stands out like a sore thumb that I’m missing?

This:

      Book.findOne({_id: bookid}, (err, data)=>{
        if(err){
            throw(err);
      //  res.send('no book exists');
          console.log(err);
        }else{
          if (!data){
            res.send("no book exists");
          }
          
          res.json(data);
        }
      })

vs. this (solution that seemingly works):


      Book
        .findOne({_id: bookid})
        .then(book => {
          if(!book){
            return res.send("no book exists");
          }else {
            return res.json(book);
          }
        })
        .catch(() => { return res.send("no book exists")});
    
    })
6

If you get it to work using a promise, that’s good. Apparently, according to the first answer in this S/O thread, until Mongoose v4 findOne didn’t return a thenable.

Knowing how to handle asynchronous behavior in as many ways as possible can only be a plus for your developer toolkit, so I would definitely try to get it working using the standard callback method. How you handle errors is up to you. It’s enough to just console.log() them with the fCC challenges/projects most of the time.

One thing to keep in mind is “One request, one response” for most response handling. In your first method, it seems that without a return in each of your if blocks, the final res.json() will always get called, which could leave you with a second attempt to set headers after sending them in one of your if blocks. You’ll likely get an error that says Can't set headers after they are sent.

7

FYI, Mongoose query promises are not really promises but thenables, you can force them into real promises by chaining exec() at the end
https://mongoosejs.com/docs/promises.html

1 Like
8

Thanks for pointing that out. I updated my response to reflect that.