{"action":"created","issue":{"url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/182","repository_url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X","labels_url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/182/labels{/name}","comments_url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/182/comments","events_url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/182/events","html_url":"https://github.com/IMA-WorldHealth/bhima-2.X/issues/182","id":139268846,"number":182,"title":"Proposal: Refactor Server Error Handling","user":{"login":"jniles","id":896472,"avatar_url":"https://avatars.githubusercontent.com/u/896472?v=3","gravatar_id":"","url":"https://api.github.com/users/jniles","html_url":"https://github.com/jniles","followers_url":"https://api.github.com/users/jniles/followers","following_url":"https://api.github.com/users/jniles/following{/other_user}","gists_url":"https://api.github.com/users/jniles/gists{/gist_id}","starred_url":"https://api.github.com/users/jniles/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jniles/subscriptions","organizations_url":"https://api.github.com/users/jniles/orgs","repos_url":"https://api.github.com/users/jniles/repos","events_url":"https://api.github.com/users/jniles/events{/privacy}","received_events_url":"https://api.github.com/users/jniles/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/labels/needs%20discussion","name":"needs discussion","color":"eb6420"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-03-08T12:43:31Z","updated_at":"2016-03-23T09:59:23Z","closed_at":null,"body":"The current state of error handling on the server is pretty poor.  Here are some problems with it:\r\n 1. The code is difficult to read.\r\n 2. We are attaching objects to the native Node `req` object on every request, even if they are never used.  This has performance implications and makes `req.codes` seem like a magic object.\r\n 3. Errors are difficult to write (e.g. `next(new req.codes.ERR_SOME_ERROR())`).\r\n 4. The error `stack` property doesn't work, making it impossible to use it to trace errors.\r\n\r\nI propose the following refactor.  Errors will be defined in files found in `server/lib/errors/*.js`.  We will implement a  subset of the error [HTTP status codes](http://www.restapitutorial.com/httpstatuscodes.html) in a few files.  Specifically, this subset:\r\n 1. `BadRequest` (400)\r\n 2. `Unauthorized` (401)\r\n 3. `Forbidden` (403)\r\n 4. `NotFound` (404)\r\n 5. `InternalServiceError` (500)\r\n\r\nThese would appear in files named like the above, for example `/server/lib/errors/NotFound.js`.  These files would contain a single function, exported via `module.exports`.  See below for a sample:\r\n```js\r\nvar util = require('util');\r\n\r\n// implement the 404 NOT FOUND error function\r\nfunction NotFound(description) {\r\n  'use strict';\r\n\r\n  // make sure we have a working stack trace\r\n  Error.captureStackTrace(this, this.constructor);\r\n\r\n  // HTTP status code and translation key for the client.\r\n  this.status = 404;\r\n  this.code = 'ERR_NOT_FOUND';\r\n\r\n  // description allows developers to attach a personalized debugging message\r\n  this.description = description || '';\r\n}\r\n\r\nutil.inherits(NotFound, Error);\r\n\r\nmodule.exports = NotFound;\r\n```\r\n\r\nA developer can then selectively import the errors they need and use them as follows:\r\n```js\r\nconst NotFound = require('../lib/errors/NotFound');\r\n\r\nexports.detail = function detail(req, res, next) {\r\n\r\n  let sql = 'SELECT some_col FROM some_table WHERE id = ?;';\r\n\r\n  db.exec(sql, [ req.params.id ])\r\n  .then(function (rows) {\r\n\r\n    // whoops!  Couldn't find the record!\r\n    if (rows.length === 0) {\r\n     throw NotFound('Could not find a record in some_table with id = ?'.replace('?', req.params.id));\r\n    }\r\n   \r\n    // all okay!  Send the response to the client\r\n    res.status(200).json(rows[0]);\r\n })\r\n .catch(next)\r\n .done();\r\n};\r\n```\r\n\r\nThis is obvious much cleaner to write and read.  We avoid most of the problems pointed out at the top of this issue, and actually get clean stack traces!\r\n\r\n[Error inheritance inspired by this gist](https://gist.github.com/justmoon/15511f92e5216fa2624b)."},"comment":{"url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/comments/200275682","html_url":"https://github.com/IMA-WorldHealth/bhima-2.X/issues/182#issuecomment-200275682","issue_url":"https://api.github.com/repos/IMA-WorldHealth/bhima-2.X/issues/182","id":200275682,"user":{"login":"jniles","id":896472,"avatar_url":"https://avatars.githubusercontent.com/u/896472?v=3","gravatar_id":"","url":"https://api.github.com/users/jniles","html_url":"https://github.com/jniles","followers_url":"https://api.github.com/users/jniles/followers","following_url":"https://api.github.com/users/jniles/following{/other_user}","gists_url":"https://api.github.com/users/jniles/gists{/gist_id}","starred_url":"https://api.github.com/users/jniles/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jniles/subscriptions","organizations_url":"https://api.github.com/users/jniles/orgs","repos_url":"https://api.github.com/users/jniles/repos","events_url":"https://api.github.com/users/jniles/events{/privacy}","received_events_url":"https://api.github.com/users/jniles/received_events","type":"User","site_admin":false},"created_at":"2016-03-23T09:59:23Z","updated_at":"2016-03-23T09:59:23Z","body":"Should I work on a PR with the errors pre-made, then developers can rebase and begin using them?"}}