{"action":"created","issue":{"url":"https://api.github.com/repos/elixir-lang/elixir/issues/5278","repository_url":"https://api.github.com/repos/elixir-lang/elixir","labels_url":"https://api.github.com/repos/elixir-lang/elixir/issues/5278/labels{/name}","comments_url":"https://api.github.com/repos/elixir-lang/elixir/issues/5278/comments","events_url":"https://api.github.com/repos/elixir-lang/elixir/issues/5278/events","html_url":"https://github.com/elixir-lang/elixir/pull/5278","id":180400892,"number":5278,"title":"Improved error messaging when Process.register/2 fails","user":{"login":"efexen","id":1451215,"avatar_url":"https://avatars.githubusercontent.com/u/1451215?v=3","gravatar_id":"","url":"https://api.github.com/users/efexen","html_url":"https://github.com/efexen","followers_url":"https://api.github.com/users/efexen/followers","following_url":"https://api.github.com/users/efexen/following{/other_user}","gists_url":"https://api.github.com/users/efexen/gists{/gist_id}","starred_url":"https://api.github.com/users/efexen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/efexen/subscriptions","organizations_url":"https://api.github.com/users/efexen/orgs","repos_url":"https://api.github.com/users/efexen/repos","events_url":"https://api.github.com/users/efexen/events{/privacy}","received_events_url":"https://api.github.com/users/efexen/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2016-09-30T20:58:16Z","updated_at":"2016-10-01T18:09:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/elixir-lang/elixir/pulls/5278","html_url":"https://github.com/elixir-lang/elixir/pull/5278","diff_url":"https://github.com/elixir-lang/elixir/pull/5278.diff","patch_url":"https://github.com/elixir-lang/elixir/pull/5278.patch"},"body":"Hey :wave: \r\n\r\nI've done a bit of work to improve the error messages when Process.register/2 fails.\r\n\r\nCurrently any of the failure cases will result in the following:\r\n\r\n ```\r\n** (ArgumentError) argument error\r\n             :erlang.register(:test, #PID<0.170.0>)\r\n    (elixir) lib/process.ex:414: Process.register/2\r\n```\r\n\r\nReading through the documentation it appears there are a couple of cases where we could make the messages more helpful.\r\n\r\nFirst is the case where we are attempting to register a process which has already finished. We can use `Process.alive?/1` to determine whether the process is still alive and provide an enhanced message:\r\n\r\n```\r\niex(1)> pid = spawn(fn() -> 1 end)\r\n#PID<0.85.0>\r\niex(2)> Process.register(pid, :test)\r\n** (ArgumentError) Process for #PID<0.85.0> is no longer alive\r\n             :erlang.register(:test, #PID<0.85.0>)\r\n    (elixir) lib/process.ex:424: Process.register/2\r\n```\r\n\r\nSecond is the case where the name has already been registered for another process. We can use `Process.whereis/1` to determine whether this is the case and provide an enhanced message:\r\n\r\n```\r\niex(1)> Process.register(self(), :test)\r\ntrue\r\niex(2)> Process.register(self(), :test)\r\n** (ArgumentError) Process with pid #PID<0.81.0> is already registered for name :test\r\n             :erlang.register(:test, #PID<0.81.0>)\r\n    (elixir) lib/process.ex:424: Process.register/2\r\n```\r\n\r\nI think both of these messages provide useful information that should make debugging the errors easier. I've also included a fallback option in case that the process is still alive and there isn't another process registered by that name it will use the original error message.\r\n\r\nI've kept the errors as type `ArgumentError` so as not to break any code that attempts to rescue these errors, do you think this is preferable to introducing a new error type?\r\n\r\nI'd love to hear your thoughts and whether we could work on this further to get it merged to provide better messages.\r\n\r\nI wasn't entirely sure what to do around things such as testing for these errors so for now I've included them in the exception tests.\r\n\r\nI have some further ideas for error message enhancements but didn't want to bundle them all here upfront without hearing some feedback first.\r\n\r\nThanks,\r\nVille"},"comment":{"url":"https://api.github.com/repos/elixir-lang/elixir/issues/comments/250927585","html_url":"https://github.com/elixir-lang/elixir/pull/5278#issuecomment-250927585","issue_url":"https://api.github.com/repos/elixir-lang/elixir/issues/5278","id":250927585,"user":{"login":"efexen","id":1451215,"avatar_url":"https://avatars.githubusercontent.com/u/1451215?v=3","gravatar_id":"","url":"https://api.github.com/users/efexen","html_url":"https://github.com/efexen","followers_url":"https://api.github.com/users/efexen/followers","following_url":"https://api.github.com/users/efexen/following{/other_user}","gists_url":"https://api.github.com/users/efexen/gists{/gist_id}","starred_url":"https://api.github.com/users/efexen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/efexen/subscriptions","organizations_url":"https://api.github.com/users/efexen/orgs","repos_url":"https://api.github.com/users/efexen/repos","events_url":"https://api.github.com/users/efexen/events{/privacy}","received_events_url":"https://api.github.com/users/efexen/received_events","type":"User","site_admin":false},"created_at":"2016-10-01T18:09:33Z","updated_at":"2016-10-01T18:09:33Z","body":"Hey @fishcakez you're absolutely right that there exists possibility of race conditions here due to the actions not being atomic but as far as I can tell hitting one of the race conditions would only result in the fallback error message which was the existing behaviour anyway?\r\n\r\nDo you have anymore details about how I could check for `:noproc` to increase the likelihood of serving the enhanced error? It looks like the call to `:erlang.register/2` always fails with `:badarg` regardless of the reason."}}
{"actor":{"display_login":"efexen"}}