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

Handling errors in request handler #52

Closed
motss opened this issue Jul 3, 2018 · 4 comments
Closed

Handling errors in request handler #52

motss opened this issue Jul 3, 2018 · 4 comments

Comments

@motss
Copy link

motss commented Jul 3, 2018

Not sure if this has been answered. I could find anything relevant here and I might have overlooked any similar issues.

This is something I encountered now.

const polka = require('polka');

polka()
  .get('/', async (req, res /** next is undefined */) => {
    // How do I handle this error? In Express, we have `next` in a request handler to forward the error, but in Polka, there is no `next` method available.
    throw new Error('test error'); // UnhandledPromiseRejectionWarning
  })
  .listen(3000);

This is what we normally would do in Express.

const express = require('express');

express()
  .get('/', async (req, res, next) => {
    try {
      const d = await doMoreStuff();
      return res.send(d);
    } catch (e) {
      return next(e); // Forward error to an error handler.
    }
  })
  .use((err, req, res, next) => {
    // Ultimately, handling error here...
  });
  .listen(3000);

Please advise on how we can deal with such situation.

@motss
Copy link
Author

motss commented Jul 3, 2018

I eventually go for something like this instead.

const polka = require('polka');

function handleError(err, req, res) {
  // handle all errors here...
}

polka()
  .get('/', async (req, res) => {
    try {
      const d = await doMoreStuff();

      send(res, 200, d);
    } catch (e) {
      handleError(e, req, res);
    }
  })
  .listen(3000);

@lukeed
Copy link
Owner

lukeed commented Jul 3, 2018

Hey,

Yeah, there's no next in the route handler -- only in middleware functions. It's possible this may change in the future, but I wouldn't count on it. I have some ideas that are related to this, but am playing with them in real applications before applying them to Polka. Everything will be solidified for the 1.0 release.

For now, my reasoning is that since you know there is an error to be thrown, you should just terminate the response directly. You can call a shared error handler as you've done, but IMO there's no reason to bubble up an error when you can just take care of it at the source. Only middleware functions have reason to bubble, since they're typically third-party and/or are handling preparatory work for your main handler & generally should not terminate the response cycle.

This is an abbreviated version of what my applications typically look like:

const { STATUS_CODES } = require('http');
const sendtype = require('@polka/send-type');

function toErr(res, code, data) {
  let error = (data && data.message) || data || STATUS_CODES[code];
  sendtype(res, code, { error });
}

function send(res, code, data) {
  (code >= 400 ? toErr : sendtype)(res, code, data);
}

// here assumes no error is ignored
//~> runs for errors thrown in middleware loop
function onError(err, req, res, _next) {
  toErr(res, err.code || err.status || 500, (err.length && err) || err.message);
}

polka({ onError })
  .get('/:id', async (req, res) => {
    try {
      let d = await Post.find(req.params.id);
      if (!d) return send(res, 404);
      send(res, 200, d);
    } catch (e) {
      send(res, 500, e);
      // or, if exported: onError(e, req, res);
    }
  })
  .listen(PORT)

@motss
Copy link
Author

motss commented Jul 4, 2018

@lukeed Thanks for the explanation and detailed code snippet. This looks like more sophisticated approach in handling errors using Polka. I learnt something today. Thank you so much. 👍

@lukeed
Copy link
Owner

lukeed commented Jul 4, 2018

No problem, glad I could help!

@lukeed lukeed closed this as completed Jul 4, 2018
lukeed added a commit that referenced this issue Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants