meteor-space / base

Foundation for Modular Application Architecture in Meteor.
MIT License
75 stars 5 forks source link

Catch unhandled exceptions in logger + prevent exiting #43

Closed rhyslbw closed 8 years ago

rhyslbw commented 8 years ago

https://github.com/winstonjs/winston#handling-uncaught-exceptions-with-winston

    this._logger = new winston.Logger({
        transports: [
          new winston.transports.Console({
            handleExceptions: true,
            colorize: true,
            prettyPrint: true
          })
        ],
        exitOnError: false
      });
DominikGuzei commented 8 years ago

Would this wrap around all thrown exceptions? I guess this is great for production, in development i would disable it as direct exceptions are often easier to debug

rhyslbw commented 8 years ago

It catches unhandled exceptions (for the purpose of logging), and optionally prevents the program exiting. Here's an example of the output to the console from a node app:


  "date": "Thu Jan 07 2016 20:46:52 GMT+1100 (AEDT)",
  "process": {
    "pid": 64786,
    "uid": 501,
    "gid": 20,
    "cwd": "/path/to/app",
    "execPath": "/usr/local/bin/node",
    "version": "v4.1.1",
    "argv": [
      "/usr/local/bin/node",
      "/path/to/app/src/app.js"
    ],
    "memoryUsage": {
      "rss": 74465280,
      "heapTotal": 57728256,
      "heapUsed": 35017656
    }
  },
  "os": {
    "loadavg": [
      6.27783203125,
      5.86572265625,
      6.11328125
    ],
    "uptime": 2075033
  },
  "trace": [
    {
      "column": 19,
      "file": "/path/to/app/src/myFile.js",
      "function": "_myFunction",
      "line": 167,
      "method": null,
      "native": false
    },
    {
      "column": 9,
      "file": "/path/to/app/src/myFile.js",
      "function": null,
      "line": 50,
      "method": null,
      "native": false
    },
    {
      "column": 15,
      "file": "fs.js",
      "function": "FSReqWrap.oncomplete",
      "line": 82,
      "method": "oncomplete",
      "native": false
    }
  ],
  "stack": [
    "ReferenceError: myVariable is not defined",
    "    at _myFunction (/path/to/app/src/myFile.js:167:19)",
    "    at /path/to/app/src/myFile.js:50:9",
    "    at FSReqWrap.oncomplete (fs.js:82:15)"
  ],
  "level": "error",
  "message": "uncaughtException: myVariable is not defined",
  "timestamp": "Thu Jan 07 2016 20:46:52 GMT+1100 (AEDT)"
}

So we should make both these settings configurable, just depends on what the defaults will be

DominikGuzei commented 8 years ago

I guess we have to test that on some real projects to get a feeling :wink:

rhyslbw commented 8 years ago

I actually think this is best not handled by the logger, as it confuses the responsibilities. That aside, preventing the program to exit after an unhandled exception is a bad idea as it leaves the app in an inconsistent state. In this case it would be best to exitOnError: true to let the process crash and then the production management tool can restart it.