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

Error handlers needed #261

Closed
5 tasks
BerkeleyTrue opened this issue Mar 24, 2015 · 4 comments
Closed
5 tasks

Error handlers needed #261

BerkeleyTrue opened this issue Mar 24, 2015 · 4 comments
Labels
type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc.

Comments

@BerkeleyTrue
Copy link
Contributor

These files need eror handling.

Proper handling tips:

  1. end early
  2. use next as callback in express middleware
  3. If there are multiple asynchronous functions, such as database queries, they must either be nested or use promise chains to resolve, preferably promises.

End Early

Whenever an error exist in express middleware, the callback function should be ended by returning the callback call with err as the first argument. What does that mean?

Down below is an example of what a proper error handling express route should look like. Notice how the function being passed into app.use method as the second argument has three parameters. This is a standard pattern in express to signify to express two things:

  1. this is handler is what is called a middleware
  2. if next, the callback for the route handler, is called by us, the camper-developer extraordinaire, with an argument, an error has occurred and needs to be handled by the error handler down the road.
app.use('/some-end-point', function(req, res, next) {
  // some really cool code happens here <<<<
  // then an async call below. Notice the callback being passed into the save method
  User.save(function (err) {
    // the save method must call the database. Lots of things can go wrong here, so if the driver that connects to the database detects something bad it will call this function that we pass into it with the error it detects. We now need to be responsible campers and deal with this error.
    if (err) {
      // if we get to this point that means `err` exists and we need to end this function and call the callback.
      return next(err);
    }
    // because of the return in the conditional above, if we reach the code below that means the database returned with no errors. Now we can proceed.
   res.send('stuff happend, saved user, end of line');
   // it is important to end the route handler in two ways. Either by calling `next` with or without an err argument. This will tell express to look for the next route handler. The other way is to send something down to the user using a `res` method. `res` here is short for response meaning the response to the user. Most of the time you will be using `res.render` but you could also call `res.end` to not send the user anything other than a message that says the request to the server has ended. 
  });
});

Here are some files where errors are not properly being handled. If you would like to help out respond here with the file that you would like call dibs on.

@BerkeleyTrue BerkeleyTrue added type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc. todo labels Mar 24, 2015
@ghost
Copy link

ghost commented Mar 26, 2015

Just to make sure I'm doing this correctly, does this look right? https://github.com/jameskopacz/freecodecamp/commit/547c1b8fab8a5cd60414a0b9e097456483331c15

@BerkeleyTrue
Copy link
Contributor Author

@jameskopacz looks good. Make the PR and I will merge. There is still one more issue with that file but that can be done in a separate PR if you want. There is a req.user.save() that needs a callback.

@ghost ghost mentioned this issue Mar 26, 2015
@ghost
Copy link

ghost commented Mar 26, 2015

Added req.user.save callback I think: #268

@aldraco
Copy link
Contributor

aldraco commented Mar 28, 2015

I can tackle the coursewares, unless someone has claimed those.

QuincyLarson pushed a commit that referenced this issue Mar 29, 2015
Coursewares error handling (issue #261)
@BerkeleyTrue BerkeleyTrue removed the todo label Apr 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Issues that need priority attention. Platform, Curriculum tests (if broken completely), etc.
Projects
None yet
Development

No branches or pull requests

2 participants