Handle/catch errors without repeating code?

Question

Consider the following (pseudo)function to update a database:

function my_function(req, res, input) {
  try {
    // This sanitizeInput() function throws error message if input is invalid
    sanitizeInput(input);
  catch (err) {
    res.status(500).json({id:"sql_error",message:err});
    return;
  }

  dbCreateRowPromise(input)
    .then(result => {//Handle success})
    .catch(err => {res.status(500).json({id:"sql_error",message:err}})
}

As you can see I'm writting catch twice and both times I'm writing the same response 500 status and handling the catch in the same way. Is there any good way to combine this 2 catches in a single one?

These types of questions don't provide a problem. This belongs on Code Review. — ProEvilz
– ProEvilz, Commented Oct 17, 2017 at 16:20
@ProEvilz I don't believe so, code review wants running code, pseudo code doesn't really interest them. — Icepickle
– Icepickle, Commented Oct 17, 2017 at 16:31
@Icepickle This question is a 'how can I improve my code' question. It doesn't matter whether it's pseudo in this instance because the highlighted part is the use of catch & try blocks. — ProEvilz
– ProEvilz, Commented Oct 18, 2017 at 9:51
@ProEvilz Actually for me, it doesn't even look like functioning code — Icepickle
– Icepickle, Commented Oct 18, 2017 at 14:39

Icepickle · Accepted Answer · 2017-10-17 16:42:02Z

1

Note that in your current way of writing the code, you are not returning anything, and any following statements will assume that everything went ok, as you are handling the catch already, and transform the response.

I think you could just rewrite your statement in the following way:

function sanitizeInput( input ) {
  if (input % 2 === 0) {
    throw 'Cannot input even number';
  }
  return input;
}

function dbCreateRowPromise( input ) {
  return Promise.resolve(input);
}

function my_function( input ) {
  return Promise.resolve(input)
    .then( sanitizeInput )
    .then( dbCreateRowPromise );
}

// perfectly normall input
my_function(5)
  .then( out => console.log(out))
  .catch( err => console.log('error occured', err));
  
// errourness input
my_function(4)
  .then( out => console.log(out))
  .catch( err => console.log('error occured', err));

Just use the promises to build a chain of events, and let the error be thrown. In case it is not handled by the callee, the error will be shown in the console eventually.

answered Oct 17, 2017 at 16:42

Icepickle

12.8k3 gold badges38 silver badges51 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

M. Lautaro Over a year ago

I first chose a different answer as the accepted one but ended up looking at this answer several times because of it's provided working code, so decided to change the accepted answer to this one as it was by far the most useful to me. Thanks =)

justin.m.chase · Accepted Answer · 2017-10-17 16:51:44Z

1

It seems like what you're encountering is a mixture of synchronous code, promises and async callbacks which have different error handling techniques.

One solution is to just do what you're doing, handle the different styles of errors individually, but this could get confusing and complicated. The other is to attempt to unify all such code into a single style, using simple abstractions to smooth them over.

I would personally attempt to normalize all of my calls as async callback style calls and then unify the error handling this way. For example it looks like you're using express middleware here, so I would pass all errors out and handle them in an error handler middleware.

import { auto, constant } from 'async'
const { assign } from Object

function sanitize (opts, callback) {
  const { input } = opts
  try {
    sanitizeInput(input)
    callback()
  } catch (err) {
    callback(assign(err, { status: 'sanitize_error', statusCode: 500 })
  }
}

function createRow (opts, callback) {
  const { santizie: input } = opts
  dbCreateRowPromise(input)
    .then(result => callback(null, result))
    .catch(err => callback(assign(err, { status: 'create_row_error', statusCode: 500 }))
}

function my_function(req, res, input, next) {
  const block = {
    input: constant(input),
    sanitize: ['input', sanitize],
    createRow: ['sanitize', createRow]
  }

  auto(block, (err, result) => {
    if (err) return next(err)
    res.status(200).json({ status: 'ok' }))
  })
}

function errorHandler(err, req, res) {
  res.status(err.statusCode).json(err)
}

answered Oct 17, 2017 at 16:51

justin.m.chase

13.7k9 gold badges61 silver badges111 bronze badges

4 Comments

Bergi Over a year ago

I don't see any asynchronous callbacks in the OPs code. Why are you going back from promise to nodeback style?

justin.m.chase Over a year ago

I didn't either but the wrapping function function my_function(req, res, input) looks like an express function which does actually have an optional implied callback. You don't get very far into express before you start using the callback also. So it may not have been explicit in the question but I inferred that it was part of the broader answer.

Bergi Over a year ago

I've always wrapped express so that it works properly with promise-returning router handlers :-)

justin.m.chase Over a year ago

I think normalizing the code as promises instead of callbacks is a totally legit option. The point is to unify them all to be the same style then use that. I personally find promises less desirable than callbacks once you have more than simple linear flow. The async library + callbacks is more elegant, imo.

Gratus D. · Accepted Answer · 2017-10-17 17:07:37Z

1

You should consider making sanitizeInput return a promise. Then you can write:

sanitizeInput(input)
.then(function(data)){
   return dbCreateRowPromise(input);
})
.then(function(data)){
   //success
})
.catch(function(error)){
   //all failures
})

I am assuming your functions return something - if not data would just be null

answered Oct 17, 2017 at 17:07

Gratus D.

8778 silver badges23 bronze badges

Comments

Bergi · Accepted Answer · 2017-10-17 16:57:42Z

The easiest way is to use async/await syntax for your function:

async function my_function(req, res, input) {
  try {
    input = sanitizeInput(input);
    let result = await dbCreateRowPromise(input);
    // handle result
  } catch (err) {
    res.status(500).json({id:"sql_error",message:err});
  }
}

Alternatively, make a promise for sanitizeInput so that you can use promise chaining and handle errors from it as a rejection.

var promise;
try {
  promise = Promise.resolve(sanitizeInput(input));
} catch (err) {
  promise = Promise.reject(err);
}
promise.then(input => 
  dbCreateRowPromise(input)
).then(result => {
  // Handle success
}).catch(err => {
  res.status(500).json({id:"sql_error",message:err});
});

or

new Promise(resolve =>
  resolve(sanitizeInput(input))
).then(input =>
  dbCreateRowPromise(input)
).then(…).catch(…);

or

Promise.resolve(input).then(sanitizeInput).then(dbCreateRowPromise).then(…).catch(…)

Collectives™ on Stack Overflow

Handle/catch errors without repeating code?

4 Answers 4

1 Comment

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related