NodeJS Unhandled Promise Rejection

2019-07-30 08:47发布

I have a method getModel(kind) that returns a Datastore data model in NodeJS.

I made a simple type-o in the getModel(kind) method and that uncovered an underlying issue with an uncaught promise rejection.

How can I update the code to catch these uncaught exceptions in the future?

Call getModel:

//Save the data to the database
getModel('transferrequest').create(TransferRequestNew, (err, savedData) => {
  if (err) {
  console.log('Transfer Request New unable to create new entity. Error message: ', err);
    next(err);
    return;
  }
  console.log('savedData: ', savedData);
  res.redirect(`${req.baseUrl}/history`);
});

getModel function:

function getModel(kind) {
  const model = __modelsdir+'/model-'+__databackend+kind;

  return require(model);
}

Create method being called with getModel.create:

function create (data, cb) {
  update(null, data, cb);
}

Unhandled Error:

(node:31937) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Cannot find module '/mypath/projectname/models/model-datastoretransferrequest'

2条回答
ら.Afraid
2楼-- · 2019-07-30 08:54

This is just a little approach of how one could solve this but there might be some better approaches. Why don't you try-catch the require statement? I have tested this and this worked fine:

function getModel(kind, cb) {

  var model = __dirname + kind;
  try {
    var requiredModel = require(model);
    return cb(null, requiredModel)
  }
  catch (e) {
    return cb(new Error("model not found!"), null);
  }
}

I have tested this with __dirname but this should not make a difference.

Btw: This shall not show a best practice Error handling, just wrote that down in my coffee break.

查看更多
做自己的国王
3楼-- · 2019-07-30 09:16

First of all, I would say that dynamic require is a way to get yourself in trouble. Let me structure my answer in two parts, to highlight the reasons why it should be avoided and propose few solutions instead.


Underlying problems

  1. Synchronous loading at the runtime

    require is a synchronous operation, and as we all know such operations block the Event Loop. Imagine a relatively huge amount of users consuming your application simultaneously, and the loop delay that surges above reasonable values or at least starts causing performance issues.

  2. Hard to test such dynamic modules

    How can you guarantee that your application is stable enough without proper test coverage? With dynamic require it becomes a non-trivial task. That leads to the other question - do you really have such amount of data models that they need to be created and loaded dynamically? And if not, you can make your work a lot more comfortable with well-defined and explicitly required data models rather than "magic" loading prone to errors. And also, don't forget about circular dependencies and other horrendous abominations, which will remain silent until the import moment.

  3. Overall design of such application

    It is worth reading ESLint topic on global-require and see why it's not allowed in various rule sets by default (Airbnb, for example). But you actually mentioned that yourself:

    I made a simple type-o in the getModel(kind) method and that uncovered an underlying issue with an uncaught promise rejection.

    If that is enough to break your application, does this application could be considered robust? It's the same thing as with raw strings as parameters: one should rather make them constants or present as function themselves, getting rid of hard-to-debug runtime errors in exchange for simple syntax errors at startup.


Possible solutions

Explicitly required models

Create well-defined object structures that represent data structures and load them explicitly from one aggregated index.js file:

// models/transfer-request.js
class TransferRequest {
  create(instance, cb) {
    // Implementation...
  }
}

module.exports = TransferRequest;

// models/index.js
const TransferRequest = require('./transfer-request.js');
// Other models requires...

module.exports = {
  TransferRequest,
  // Other models exports...
};

Create instances using Factory:

If you still want to stick with something similar to you've got now, create a model Factory using standard pattern:

class ModelFactory {
  constructor(model){
    switch(model) {
      case 'transferrequest':
        // Implementation of TransferRequest object creation...
      break;

      case 'othertransferrequest':
        // Implementation of OtherTransferRequest object creation...
      break;

      default:
        throw new Error('Unknown class');
    }
  }
}

It is times and times better to create actual model objects in clear and visible way, well-defined in code than obscuring possible options with dynamic loading.

And yes, if you still want require at a runtime, just use try/catch to handle rejections.

查看更多
登录 后发表回答