{"action":"created","issue":{"url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/1105","labels_url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/1105/labels{/name}","comments_url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/1105/comments","events_url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/1105/events","html_url":"https://github.com/mozilla/fxa-auth-server/pull/1105","id":115107459,"number":1105,"title":"fix(server): refactor account promise chains to named functions","user":{"login":"philbooth","id":64367,"avatar_url":"https://avatars.githubusercontent.com/u/64367?v=3","gravatar_id":"","url":"https://api.github.com/users/philbooth","html_url":"https://github.com/philbooth","followers_url":"https://api.github.com/users/philbooth/followers","following_url":"https://api.github.com/users/philbooth/following{/other_user}","gists_url":"https://api.github.com/users/philbooth/gists{/gist_id}","starred_url":"https://api.github.com/users/philbooth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/philbooth/subscriptions","organizations_url":"https://api.github.com/users/philbooth/orgs","repos_url":"https://api.github.com/users/philbooth/repos","events_url":"https://api.github.com/users/philbooth/events{/privacy}","received_events_url":"https://api.github.com/users/philbooth/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/mozilla/fxa-auth-server/labels/waffle:review","name":"waffle:review","color":"ededed"}],"state":"open","locked":false,"assignee":{"login":"dannycoates","id":87619,"avatar_url":"https://avatars.githubusercontent.com/u/87619?v=3","gravatar_id":"","url":"https://api.github.com/users/dannycoates","html_url":"https://github.com/dannycoates","followers_url":"https://api.github.com/users/dannycoates/followers","following_url":"https://api.github.com/users/dannycoates/following{/other_user}","gists_url":"https://api.github.com/users/dannycoates/gists{/gist_id}","starred_url":"https://api.github.com/users/dannycoates/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dannycoates/subscriptions","organizations_url":"https://api.github.com/users/dannycoates/orgs","repos_url":"https://api.github.com/users/dannycoates/repos","events_url":"https://api.github.com/users/dannycoates/events{/privacy}","received_events_url":"https://api.github.com/users/dannycoates/received_events","type":"User","site_admin":false},"milestone":null,"comments":6,"created_at":"2015-11-04T17:40:51Z","updated_at":"2015-11-05T06:08:39Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mozilla/fxa-auth-server/pulls/1105","html_url":"https://github.com/mozilla/fxa-auth-server/pull/1105","diff_url":"https://github.com/mozilla/fxa-auth-server/pull/1105.diff","patch_url":"https://github.com/mozilla/fxa-auth-server/pull/1105.patch"},"body":"This is the change that I described in https://github.com/mozilla/fxa-auth-server/pull/1102#issuecomment-153658657.\r\n\r\nThe background is that I find the long promise chains on some of the route handlers very difficult to follow. For instance, I'll be scrolling down one of them, reading it, and suddenly I'll be unsure whether I'm still in the same handler or if I've scrolled into the next one. Cue scrolling up to check and then scrolling back down to try and find where I was before I scrolled up. Sometimes this happens more than once in a row.\r\n\r\nAlso the constant in-then-out indentation just makes it hard to follow the flow of logic through the promise chain, figuring out where a given `return` or `throw` ends up is trickier than it needs to be. Which level of indentation is this `then` at? Scroll up, scroll back down, etc.\r\n\r\nAll I've done is extract each of the inline functions out to a named function in the same context. Doing so leaves behind a high-level promise chain that looks like the following (for `/v1/account/create`):\r\n\r\n```js\r\ncustoms.check(request.app.clientAddress, email, 'accountCreate')\r\n  .then(db.emailRecord.bind(db, email))\r\n  .then(deleteAccount, ignoreUnknownAccountError)\r\n  .then(checkPreVerified)\r\n  .then(createPassword)\r\n  .then(createAccount)\r\n  .then(createSessionToken)\r\n  .then(sendVerifyCode)\r\n  .then(createResponse)\r\n  .done(sendResponse, reply)\r\n```\r\n\r\n...instead of the 131-line behemoth that was there previously.\r\n\r\nA little bit of local state (5 vars) is added to make this possible but it's a small price, worth paying I think. What do you guys reckon, @rfk & @dannycoates?"},"comment":{"url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/comments/153965915","html_url":"https://github.com/mozilla/fxa-auth-server/pull/1105#issuecomment-153965915","issue_url":"https://api.github.com/repos/mozilla/fxa-auth-server/issues/1105","id":153965915,"user":{"login":"dannycoates","id":87619,"avatar_url":"https://avatars.githubusercontent.com/u/87619?v=3","gravatar_id":"","url":"https://api.github.com/users/dannycoates","html_url":"https://github.com/dannycoates","followers_url":"https://api.github.com/users/dannycoates/followers","following_url":"https://api.github.com/users/dannycoates/following{/other_user}","gists_url":"https://api.github.com/users/dannycoates/gists{/gist_id}","starred_url":"https://api.github.com/users/dannycoates/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dannycoates/subscriptions","organizations_url":"https://api.github.com/users/dannycoates/orgs","repos_url":"https://api.github.com/users/dannycoates/repos","events_url":"https://api.github.com/users/dannycoates/events{/privacy}","received_events_url":"https://api.github.com/users/dannycoates/received_events","type":"User","site_admin":false},"created_at":"2015-11-05T06:08:39Z","updated_at":"2015-11-05T06:08:39Z","body":"> further thoughts on the performance or other aspects of this?\r\n\r\nI'm sure the performance will be fine. The one nit I have is a stupid style thing... \r\n\r\n```\r\n.then(function () {\r\n\r\n})\r\n```\r\n\r\nSneaking the function definition on the same line as the `then`. I know **everyone** does it that way but I think something as important as a new function should be prominent. A new line makes it stand out more as a distinct \"thing\" instead of some archaic js syntax quirk. \r\n\r\nIf anyone ever runs out of newlines, I've got a whole bag to share :)"}}