Community
Participate
Working Groups
Steps: 1. Clone the same repo twice 2. In the first one create a branch and push it 3. In the second one, fetch remote changes, assure the new branch is there 4. In the first one, remove the branch (push the change) 5. In the second one, try to fetch changes for the removed branch => IS: 500, Remote does not have refs/heads/test available for fetch. => SHOULD BE: 404, Remote does not have refs/heads/test available for fetch.
Are you sure that it should be 404? Remote branch still exists, it's some kind of resource, you can find it at refs/remotes/branch_name in .git directory. However fetching it causes error, as given remote doesn't have it. So, imo 404 is not apprioprate in that case. I would return 404 i.e., when you try to fetch remote branch, which can't found at refs/remotes/branch_name.
(In reply to comment #1) > Are you sure that it should be 404? Now, when you brought that up, I'm so sure about it. I agree 404 is not the best choice. I'm in two minds about the proper response, whether it should 200 with a message or 405 (Method Not Allowed). What do you think?
I think that these HTTP codes apply to quite simple cases, like resource is not found (404), or method (GET/POST etc.) is not appropriate for the resource and server doesn't know what to do. In this case, resource is found and method is appropriate (as it launches fetching). Error is thrown from "deep" Orion/JGit internals, as case is quite complex and git-specific. I think that 500 is fits well, however it looks bad. ;)
(In reply to comment #3) >I think that 500 is fits well, however it looks bad. ;) Well, imo what we're dealing here with is not an "Internal Server Error". It's an exception thrown by a lower layer i.e. JGit, but this is how they (usually) indicate an "exceptional" case. It doesn't mean we're not able to recover from that. Moreover, I don't think we should propagate all these exceptions to the user in the form of an error.
Error is cause by JGitInternalException, which has cause: TransportException with quite pretty detailed message: "Remote does not have refs/heads/a available for fetch." Another code instead of 500? Ok, but how do you want to distinguish this situation? JGitInternalExcetion and TransportException aren't very descriptive and one-situation specific. What could we do? Detailed message is displayed. For me, in that case, situation is clear for user and I can't see better solution.
(In reply to comment #5) > For me, in that case, situation is clear for user and I can't see better solution. Agreed. Just like in bug 346285, let's assume 500 is fine as long as a meaningful error message is presented to the user.
