{"action":"created","issue":{"url":"https://api.github.com/repos/libgit2/libgit2/issues/3257","labels_url":"https://api.github.com/repos/libgit2/libgit2/issues/3257/labels{/name}","comments_url":"https://api.github.com/repos/libgit2/libgit2/issues/3257/comments","events_url":"https://api.github.com/repos/libgit2/libgit2/issues/3257/events","html_url":"https://github.com/libgit2/libgit2/pull/3257","id":91037577,"number":3257,"title":"Errors: drop the error \"class\", use a string instead of a `git_error` structure","user":{"login":"ethomson","id":1130014,"avatar_url":"https://avatars.githubusercontent.com/u/1130014?v=3","gravatar_id":"","url":"https://api.github.com/users/ethomson","html_url":"https://github.com/ethomson","followers_url":"https://api.github.com/users/ethomson/followers","following_url":"https://api.github.com/users/ethomson/following{/other_user}","gists_url":"https://api.github.com/users/ethomson/gists{/gist_id}","starred_url":"https://api.github.com/users/ethomson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ethomson/subscriptions","organizations_url":"https://api.github.com/users/ethomson/orgs","repos_url":"https://api.github.com/users/ethomson/repos","events_url":"https://api.github.com/users/ethomson/events{/privacy}","received_events_url":"https://api.github.com/users/ethomson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2015-06-25T18:14:52Z","updated_at":"2015-06-26T15:41:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/libgit2/libgit2/pulls/3257","html_url":"https://github.com/libgit2/libgit2/pull/3257","diff_url":"https://github.com/libgit2/libgit2/pull/3257.diff","patch_url":"https://github.com/libgit2/libgit2/pull/3257.patch"},"body":"So...  I've never really found the error classes particularly useful.  At some point I was talking to someone (I think @carlosmn ?) and mentioned this and we talked about removing them.  Here's a spike to do so, and now we give callers error messages (in the form of a string) instead of a struct that has an integer that they likely won't do anything with.\r\n\r\nAlso moved the actual error message to be in a `git_buf` per TLS, which hopefully reduces the number of allocations that we do (we set error messages a lot that never wind up going to the caller.)\r\n\r\nOutstanding questions while I'm breaking shit:\r\n\r\n* Why `giterr`...  All our other everythings are `git_` prefixed - should this be `git_err` or `git_error`?\r\n* The likely useful class was probably the one that indicated OOM.  Should there be a `GIT_ENOMEM` or similar return code?"},"comment":{"url":"https://api.github.com/repos/libgit2/libgit2/issues/comments/115735481","html_url":"https://github.com/libgit2/libgit2/pull/3257#issuecomment-115735481","issue_url":"https://api.github.com/repos/libgit2/libgit2/issues/3257","id":115735481,"user":{"login":"carlosmn","id":335443,"avatar_url":"https://avatars.githubusercontent.com/u/335443?v=3","gravatar_id":"","url":"https://api.github.com/users/carlosmn","html_url":"https://github.com/carlosmn","followers_url":"https://api.github.com/users/carlosmn/followers","following_url":"https://api.github.com/users/carlosmn/following{/other_user}","gists_url":"https://api.github.com/users/carlosmn/gists{/gist_id}","starred_url":"https://api.github.com/users/carlosmn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/carlosmn/subscriptions","organizations_url":"https://api.github.com/users/carlosmn/orgs","repos_url":"https://api.github.com/users/carlosmn/repos","events_url":"https://api.github.com/users/carlosmn/events{/privacy}","received_events_url":"https://api.github.com/users/carlosmn/received_events","type":"User","site_admin":false},"created_at":"2015-06-26T15:41:20Z","updated_at":"2015-06-26T15:41:20Z","body":"I certainly agree that we could have a lot more error codes. We've been limiting ourselves to codes where we expect the caller to be able to perform some specific recovery beyond passing the error string on to its caller or the user, rather than as a way to provide details about the error.\r\n\r\nWould you do away with the free-form error string and rely on this `git_errno` and `git_strerror()` to provide the error string? Or would you still like to keep it?\r\n\r\nWhere I see the classes be useful is for complex operation, a `ENOTFOUND` doesn't quite tell you *what* wasn't found, whereas the class can tell you whether it was an index entry or a reference or whatever. I don't know if this is something anybody making use of. I personally don't for the code we have using libgit2.\r\n\r\nI'd like to also point to #3036 which is an example of how to solve a somewhat different problem, while still related to how we handle errors. As we allow pluggable bits, there's times where the caller and a backend want to exchange extra information about an error, but we don't currently allow anything other than returning the string and a pre-defined error code."}}