{"action":"created","issue":{"url":"https://api.github.com/repos/sequelize/sequelize/issues/6116","repository_url":"https://api.github.com/repos/sequelize/sequelize","labels_url":"https://api.github.com/repos/sequelize/sequelize/issues/6116/labels{/name}","comments_url":"https://api.github.com/repos/sequelize/sequelize/issues/6116/comments","events_url":"https://api.github.com/repos/sequelize/sequelize/issues/6116/events","html_url":"https://github.com/sequelize/sequelize/issues/6116","id":160459297,"number":6116,"title":"Prepare for coroutines","user":{"login":"overlookmotel","id":557937,"avatar_url":"https://avatars.githubusercontent.com/u/557937?v=3","gravatar_id":"","url":"https://api.github.com/users/overlookmotel","html_url":"https://github.com/overlookmotel","followers_url":"https://api.github.com/users/overlookmotel/followers","following_url":"https://api.github.com/users/overlookmotel/following{/other_user}","gists_url":"https://api.github.com/users/overlookmotel/gists{/gist_id}","starred_url":"https://api.github.com/users/overlookmotel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/overlookmotel/subscriptions","organizations_url":"https://api.github.com/users/overlookmotel/orgs","repos_url":"https://api.github.com/users/overlookmotel/repos","events_url":"https://api.github.com/users/overlookmotel/events{/privacy}","received_events_url":"https://api.github.com/users/overlookmotel/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/sequelize/sequelize/labels/discussion","name":"discussion","color":"d4c5f9"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":25,"created_at":"2016-06-15T15:58:23Z","updated_at":"2016-07-13T15:32:32Z","closed_at":null,"body":"Now that we're all ES6 for Sequelize v4, the door is open to be using coroutines. Personally, I think this is a great thing - it'll simplify Sequelize's codebase a *lot*.\r\n\r\nHowever, I don't think we're quite ready to do that - I'd like to conclude my research on bluebird and CLS, to make absolutely sure that introducing coroutines won't break CLS support, before we take the plunge.\r\n\r\nBut when we do start using coroutines, it will be a semver major change, due to a change in the way errors are handled.\r\n\r\n#### Error handling\r\n\r\nAt present some Sequelize async methods throw synchronously if the arguments they are passed are illegal in some way. Once those methods become coroutines, this wouldn't be the case any more - they'll return rejected promises instead.\r\n\r\nHow it is now:\r\n\r\n```js\r\nModel.findAll = function(options) {\r\n    if (!options) throw new Error('You must provide options!');\r\n}\r\n\r\n// Model.findAll() throws synchronously\r\ntry {\r\n   Model.findAll();\r\n} catch (err) {\r\n   console.log('It throws!');\r\n}\r\n```\r\n\r\nHow it will be with coroutines:\r\n\r\n```js\r\nModel.findAll = co.wrap( function*(options) {\r\n    if (!options) throw new Error('You must provide options!');\r\n} );\r\n\r\n// Model.findAll() returns a rejected promise\r\nModel.findAll().catch( function(err) {\r\n    console.log('It returns a rejected promise');\r\n} );\r\n```\r\n\r\n#### A proposal to \"prepare the way\"\r\n\r\nI would like to see coroutines in Sequelize as soon as possible, but I don't want to hold up the release of v4 until the CLS issues are resolved.\r\n\r\nI propose that we \"prepare the way\" now by converting all Sequelize methods from throwing synchronously to returning rejected promises.\r\n\r\nThis isn't hard to do using `Promise.method()`:\r\n\r\n```js\r\nModel.findAll = Promise.method( function(options) {\r\n    if (!options) throw new Error('You must provide options!');\r\n} );\r\n\r\n// Model.findAll() returns a rejected promise\r\nModel.findAll().catch( function(err) {\r\n    console.log('It returns a rejected promise');\r\n} );\r\n```\r\n\r\nThis way:\r\n\r\n1. The whole API switches away from sync throwing in one go.\r\n2. It can all be done as a single change in Sequelize v4.0.0.\r\n3. Methods can then be refactored with coroutines as and when we feel like it, without creating any further breaking changes.\r\n4. We won't have to wait until Sequelize v5 to start using coroutines.\r\n\r\nWhat do you think?"},"comment":{"url":"https://api.github.com/repos/sequelize/sequelize/issues/comments/232393149","html_url":"https://github.com/sequelize/sequelize/issues/6116#issuecomment-232393149","issue_url":"https://api.github.com/repos/sequelize/sequelize/issues/6116","id":232393149,"user":{"login":"overlookmotel","id":557937,"avatar_url":"https://avatars.githubusercontent.com/u/557937?v=3","gravatar_id":"","url":"https://api.github.com/users/overlookmotel","html_url":"https://github.com/overlookmotel","followers_url":"https://api.github.com/users/overlookmotel/followers","following_url":"https://api.github.com/users/overlookmotel/following{/other_user}","gists_url":"https://api.github.com/users/overlookmotel/gists{/gist_id}","starred_url":"https://api.github.com/users/overlookmotel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/overlookmotel/subscriptions","organizations_url":"https://api.github.com/users/overlookmotel/orgs","repos_url":"https://api.github.com/users/overlookmotel/repos","events_url":"https://api.github.com/users/overlookmotel/events{/privacy}","received_events_url":"https://api.github.com/users/overlookmotel/received_events","type":"User","site_admin":false},"created_at":"2016-07-13T15:32:32Z","updated_at":"2016-07-13T15:32:32Z","body":"@mickhansen ???\r\n\r\nIn truth I have doubted my sanity in the lengths I've ended up going to get cls-bluebird working (and the stupid amount of tests). But it's one of those things where once you've got half way, you might as well continue to the end..."}}
{"actor":{"display_login":"overlookmotel"}}