operations: Return proper error codes from UserMergeBranch
When updating the target reference fails in the UserMergeBranch RPC, then we return an error which is embedded in the response message instead of returning a proper gRPC error with an error code. This behaviour was ported over from the old Ruby implementation, but it's not considered good practice. What makes the problem worse though is that in case git-update-ref(1) fails because of a racy update, then we neither return a gRPC error nor do we set an error message in the response. As a result, the caller can only discern this case by checking whether the response is empty.
Fix this issue by returning proper error codes in all cases and deprecating the in-struct error message.
Fixes #3582 (closed)
Merge request reports
Activity
changed milestone to %14.2
added devopscreate groupgitaly sectiondev typebug workflowready for review labels
assigned to @pks-t
requested review from @stanhu
1 Warning This merge request is definitely too big (2651 lines changed), please split it into multiple merge requests. Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.
Category Reviewer Maintainer ~ Sami Hiltunen ( @samihiltunen
) (UTC+2, same timezone as@pks-t
)Sami Hiltunen ( @samihiltunen
) (UTC+2, same timezone as@pks-t
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by GitalyBotadded 1 commit
- 78e738fb - operations: Return proper error codes from UserMergeBranch
@stanhu This MR adjust UserMergeBranch to always return proper error codes, but it'll likely require changes to Rails. The previously empty error case where updateref fails now returns a PreconditionFailed error code, while the case where hooks fail will return PermissionDenied.
@pks-t Looks good to me, thanks! It sounds like we should mark this as
Draft
until we get the Rails changes merged? Were you going to update the Rails code?
Setting label(s) Category:Gitaly based on groupgitaly.
added Category:Gitaly label
requested review from @samihiltunen and @8bitlife
- Automatically resolved by Patrick Steinhardt
- Automatically resolved by Patrick Steinhardt
105 var updateRefError updateref.Error 106 107 if errors.As(err, &preReceiveError) { 108 err = stream.Send(&gitalypb.UserMergeBranchResponse{ 109 PreReceiveError: preReceiveError.Message, 110 }) 111 } else if errors.As(err, &updateRefError) { 112 // When an error happens updating the reference, e.g. because of a race 113 // with another update, then Ruby code didn't send an error but just an 114 // empty response. 115 err = stream.Send(&gitalypb.UserMergeBranchResponse{}) 106 if errors.As(err, &updateref.PreReceiveError{}) { 107 // PreReceiveErrors indicate that any of the hooks rejected the update. This 108 // typically indicates that the caller tried to update a reference in a way 109 // that is prohibited. 110 return status.Error(codes.PermissionDenied, err.Error()) Question: @8bitlife @pks-t I've just discovered that we've just added an additional error handling for merge conflicts in https://gitlab.com/gitlab-org/gitaly/blob/780588a55b9219f3157cc984f7e1b7aa9f9124f2/internal/gitaly/service/operations/merge.go#L83.
Failed to merge for source_sha %s into target_sha %s"
doesn't seem to say that it was failed due to a merge conflict and it usescodes.FailedPrecondition
. Unless we usecodes.FailedPrecondition
exclusively for merge conflicts in the context ofUserMergeBranch
, above error message doesn't seem to be clear enough. I'd like to avoid yet another string match to add that information from Rails' side.@8bitlife Was there any reason the language of
due to conflicts
was dropped from the change you've made in the above change? I'd think we could've just kept that so that we can just display that in the UI. Am I missing something?I feel
PreReceiveError
would have similar issue since we are droppingpreReceiveError
value and returning an error code with a message instead. I'm not sure if we can directly map that toGitlab::Git::PreReceiveError
from Rails' side unlesscodes.PermissionDenied
is used only for that purpose.It's unclear to me if we're expecting Rails' to display all errors to UI as is except the ones with
codes.Internal
. If that's the case, I think we need to update the error message for merge conflicts as I mentioned above.Please let me know if I missed anything obvious as I'm not very familiar with this part of our codebase.
I feel PreReceiveError would have similar issue since we are dropping preReceiveError value and returning an error code with a message instead. I'm not sure if we can directly map that to Gitlab::Git::PreReceiveError from Rails' side unless codes.PermissionDenied is used only for that purpose.
@dskim_gitlab For now I don't think we use
codes.PermissionDenied
anywhere else, so we should be good here. Sometimes I wish that we invoked/internal/allowed
not via HTTP, but via gRPC, too. In that case, we could just take a hands-off approach and directly return the error from Rails including the error code that Rails has set. This would fix the issue we have right now, where the original error gets raised in Rails, then converted by Gitaly and ultimately ends up in Rails again, but now in a different form.It's unclear to me if we're expecting Rails' to display all errors to UI as is except the ones with codes.Internal. If that's the case, I think we need to update the error message for merge conflicts as I mentioned above.
Honestly, I'm not really familiar with how errors are handled by Rails either. Might be worth it to pull in a domain expert in Rails? Maybe @proglottis knows something here?
@dskim_gitlab @pks-t Sorry. I'm not sure why I didn't see these mentions earlier.
The way errors are handled in rails is a mixed bag, spread out all over the place. So it wouldn't surprise me if we're giving the user too much or too little on a case by case basis. The reason
PreReceiveError
are shown to users is because it has it's own sanitation code, it requires a special "safe" prefix https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/git/pre_receive_error.rb#L13-14Maybe using special prefixes will work here too but we really shouldn't rely on plaintext errors from gitaly. They will likely never be localised correctly so displaying them to the user should be a last ditch effort, not to mention the possibility of leaking sensitive data.
I think especially input validation errors should probably be checked by ruby before the RPC call, or otherwise maybe we need to somehow introduce a structured error that could be interpreted better and run through i18n.
@proglottis Yeah, I agree that we would probably want to run it through i18n so it won't be ideal to just display how it was provided from Gitaly. I saw that special safe prefix as well, but didn't seem to be used in many places.
I wonder if Gitaly should return more precise git
error_code
instead of plain text error messages? I feel that would be easier to parse from Rails and have proper i18n text for it. I was hoping we could somehow use GRPC error codes for that. However, it doesn't seem that the available set of error types are not enough to cover all cases.I'm not sure if it's possible to raise an exception with additional field though. If not, should we add error_codes as a prefix to the error message instead?
Edited by Sincheol (David) KimI'm not sure if it's possible to raise an exception with additional field though. If not, should we add error_codes as a prefix to the error message instead?
I don't think it is.
In this context we've got two categories of errors:
- Any error which happens in Gitaly itself, or which happens when POSTing to
/internal/allowed
. These shouldn't ever be printed to the user. - Any error message returned by GitLab's access checks after we have successfully POSTed to
/internal/allowed
. These are fully under Rails' control, and Gitaly should pass them through without touching the error message.
In Gitaly, we already prefix the second case with the "GitLab: " safe-prefix, but indeed we also do so for some errors which shouldn't be visible to the user. This needs fixing in Gitaly such that we only do so in case we've got a message from Rails.
If we fix handling of this prefix in Gitaly to add it if and only if the error message is from Rails, would that help?
- Any error which happens in Gitaly itself, or which happens when POSTing to
@pks-t Yeah. If gitaly consistently used a prefix to signify "safe error to give to user" then that'd be much easier to handle.
In this case I think we still need a permission denied handler on the rails side to provide a generic permission denied error to the user assuming we find no special prefix.
@pks-t I agree consistent behaviour would help working with gitaly errors to avoid confusions and possible disclosure of internal error messages.
Are we using
GitLab:
prefix only for the error messages that came from GitLab itself as in/internal/allowed
as in it means the error messageGitLab
gave to gitaly?I'm talking more about the kind of error messages provided by gitaly itself like merge conflicts. I believe that's handled in https://gitlab.com/gitlab-org/gitaly/blob/780588a55b9219f3157cc984f7e1b7aa9f9124f2/internal/gitaly/service/operations/merge.go#L83 so this MR may not be directly related to that. However, I wonder how we should handle this from Rails side reliably if we can't safely infer that it was indeed a merge conflict nor can we display the error message as is due to the possible safety concern of its content.
I feel it'd be nice if gitaly can return machine friendly error codes which Rails can map to appropriate error messages with proper i18n support. Having an error prefix could be an alternative solution. Something like
ERROR:merge_conflict: Failed to merge for source_sha %s into target_sha %s
. It'd be cumbersome to catch all GRPC exceptions and matching error messages though. It seems more appealing to have it return empty response with a particular error_code now instead of having to rescue all those exceptions and inspecting error messages.In this case I think we still need a permission denied handler on the rails side to provide a generic permission denied error to the user assuming we find no special prefix.
@proglottis Yes, I think we need additional changes for this MR to work if we're going ahead with this change. I've hijacked the conversation in this MR as it didn't seem to solve the issue I was looking into.
Are we using GitLab: prefix only for the error messages that came from GitLab itself as in /internal/allowed as in it means the error message GitLab gave to gitaly?
@dskim_gitlab That's at least the intent, even though it's not currently implmented like this.
I'm talking more about the kind of error messages provided by gitaly itself like merge conflicts. I believe that's handled in https://gitlab.com/gitlab-org/gitaly/blob/780588a55b9219f3157cc984f7e1b7aa9f9124f2/internal/gitaly/service/operations/merge.go#L83 so this MR may not be directly related to that. However, I wonder how we should handle this from Rails side reliably if we can't safely infer that it was indeed a merge conflict nor can we display the error message as is due to the possible safety concern of its content.
Right. It really shows that the gRPC errors alone are not a good fit. As @proglottis said before, what we really need are structured errors such that we can tell Rails what exactly has gone wrong, ideally via gRPC messages which are embedded in the error itself.
There is for example the errdetails package, which allows us to append additional info to errors in a semi-structured way. It would still require parsing of strings though and is thus fragile.
But more generally, the gRPC error model allows for attaching arbitrary message types to gRPC errors. That looks like a much better fit to me in this context, where we can define RPC-specific error gRPC messages with structured data. In the context of UserMergeBranch, we can for example use the following errors:
message AccessCheckError { message Change { bytes old, new, ref; } bytes message []; repeated Change changes; } message MergeConflictError { bytes ours, theirs, base; repeated bytes conflicting_paths; }
On Rails side, you'd have to check whether error details contain any such structs and then generate error messages from them.
One limitation is that we must restrict error sizes to 2kB at max according to the documentation given that errors are embedded in the header. But it's still a lot better to what we have right now, I'd say.
Edited by Patrick SteinhardtBut more generally, the gRPC error model allows for attaching arbitrary message types to gRPC errors. That looks like a much better fit to me in this context, where we can define RPC-specific error gRPC messages with structured data. In the context of UserMergeBranch, we can for example use the following errors:
Ok, that might be better indeed if we can send structured data along with the error. I wonder how well that would work if all several gRPC error types could potentially have different kind of messages tagged with them though
Anyways, I'd love to try it out. Would you be able to create an WIP MR for that so that I could test it out from Rails?
I wonder how well that would work if all several gRPC error types could potentially have different kind of messages tagged with them though.
@dskim_gitlab Yeah, this is something I had been wondering, too, it might get unwieldy on Rails side. I thus wondered whether we should instead introduce a per-RPC errror type. Something like the following:
message UserMergeBranchError { enum Type { PRE_RECEIVE_ERROR = 1; MERGE_CONFLICT = 2; }; message MergeConflictError { bytes ours = 1 bytes theirs = 2 bytes base = 3; repeated bytes conflicting_files = 4; }; message PreReceiveErorr { bytes message = 1; }; Type type = 1; MergeConflictError merge_conflict = 2; PreReceiveError pre_receive = 3; }
It's a lot more unwieldy to define, especially so given that quite a lot of these will be repeated across different RPCs. But on Rails side, it's easier to handle given that you know which RPC may return which error details and aren't left guessing. WDYT?
Edited by Patrick SteinhardtIt's a lot more unwieldy to define, especially so given that quite a lot of these will be repeated across different RPCs. But on Rails side, it's easier to handle given that you know which RPC may return which error details and aren't left guessing. WDYT?
I think having clearly defined list of possible errors would be very helpful. I feel some kind of
error_code
defined asenum
with a list of possible errors and human a readable error message could be enough? I'm not sure if we are going to need those extra data for generating error message to the end user.I feel some kind of error_code defined as enum with a list of possible errors and human a readable error message could be enough?
@dskim_gitlab I don't think Gitaly should be in a position to generate any message at all, except of course the error message we already generate right now. But these are rather intended for developers such that we can easily find the location where it's generated via breadcrumbs.
For Rails/users, my plan is not to generate any messages at all in Gitaly, but instead to give Rails the information needed to generate messages. E.g. with above structs, if you see a
MergeConflictError
, then you can easily generate a message by taking the conflicting files from that error as well as the conflicting versions.Edited by Patrick SteinhardtYes, by error message I meant debuggable message intended for developer as we currently do if that helps. I think receiving some kind of error type would be enough and the extra information would be nice, but I wonder if that makes things more complicated if they are different per error type.
changed this line in version 3 of the diff
@dskim_gitlab @8bitlife @stanhu @zj-gitlab I have reworked this MR to implement above as a proof of concept. Instead of embedding PreReceiveErrors in the response, we make use of the richer gRPC error model which allows us to pass arbitrary error details in the status proto. In the context of UserMergeBranch, we thus have a new UserMergeBranchError message which embeds (for now) only a generic AccessCheckError message:
// AccessCheckError is an error returned by GitLab's `/internal/allowed` // endpoint. message AccessCheckError { // ErrorMessage is the error message as returned by the endpoint. string error_message = 1; // Protocol is the protocol used. string protocol = 2; // UserId is the user ID as which changes had been pushed. string user_id = 3; // Changes is the set of changes which have failed the access check. bytes changes = 4; } // UserMergeBranchError includes error descriptions which may be set as error // details in case UserMergeBranch fails. message UserMergeBranchError { // AccessCheckError is set if the RPC failed because `/internal/allowed` failed. AccessCheckError access_check = 1; }
The
AccessCheckError
is intended to be shared across different RPCs for each RPC which performs access checks, whileUserMergeBranchError
s are only used byUserMergeBranch
. This allows us to have generic handling for specific errors while making it easy to discover which errors a specific RPC can return. With multiple errors being available, Rails can then check which of the fields is set to deduce the correct error.For now, this is still quite limited given that we only treat access check errors. As I said above, we'll want to extend this to also support for example
MergeConflictErrors
which tell Rails for example which merge failed and what files have conflicts. I wanted to keep it simple for now though to make it easier to iterate if there's any design concerns.Let me know what you thing.
Edited by Patrick SteinhardtThanks a lot for that @pks-t ! I'll play with it tomorrow to see how it looks from the ruby side, but it looks reasonable to me.
What do you think about adding
error_type
as an enum though? It'd be ideal iferror_type
returns AccessCheckError message itself, but I'm not sure if that's possible in go. I think it might be easier to inspect when we have several error types asUserMergerBranchError
as you just have to check theerror_type
once and check the message as opposed to checking all error messages subsequently until you find a match.What do you think about adding error_type as an enum though?
@dskim_gitlab I was torn on whether to add it or not. Ultimately the enum will need to be per RPC as RPCs may have errors which are specific to it, so a single global enum doesn't really work. But if we have per-RPC enums, then it's not a whole lot better to use compared to just checking whether fields are set.
It'd be ideal if error_type returns AccessCheckError message itself, but I'm not sure if that's possible in go.
This is not specific to Go, but to gRPC/protobuf.
Maybe we would be better served if we make the error messages a
oneof
? That more or less is exactly what we're doing, which is that we return one of a set of different error details. I'm not sure whether Ruby-side tooling makes them enjoyable to use, or if anenum
would still be better in that case. Judging by the documentation, it doesn't look like it would significantly help though.@pks-t Yes,
oneof
sounds like what we're looking for. It guarantees that there is only one error message type and it gives us the symbol which we can then use to fetch retrieve the data we need.@dskim_gitlab Changed to a
oneof
nowThanks @pks-t
I'm not sure if I'm doing something wrong, but this is all I get when I raise that error from gitaly.
I was expecting more structured hash, but I can't find anything else available from https://github.com/grpc/grpc/blob/master/src/ruby/lib/grpc/errors.rb
@dskim_gitlab The status you've got has the
details
field set to aGoogle::Protobuf::Any
. You'll need to unmarshal the field to extract the structured error, where its type usUserMergeBranchError
.Thanks @pks-t! I can see that I can get error object if I do this.
I was expecting this was decoded as part of
GRPC::PermissionDenied
above somehow.Does this mean we need to check
e.to_rpc_status.details.first[:type_url]
for any GRPC error, sinceUserMergeBranchError
could be embedded with any GRPC error, and see if it has matching string ofUserMergeBranchError
and then decode it to get the structured data from it?Also,
details
seems to be an array so I wonder what should happen if we have more than one item in thereI hope I'm missing something, but it seems a little too cumbersome to me.
@stanhu @proglottis Do you have any thoughts on this? I guess we should move forward with it if this is the best we can do, but I'm unsure if we're making more complicated than it could be as I have very limited knowledge around this. I personally fill like attaching this kind of message to an empty response seems more appealing than utilising
GRPC
error like this though, but it may not be the best practice.Does this mean we need to check e.to_rpc_status.details.first[:type_url] for any GRPC error, since UserMergeBranchError could be embedded with any GRPC error, and see if it has matching string of UserMergeBranchError and then decode it to get the structured data from it?
@dskim_gitlab Not sure about Ruby, but in Go you have nice helper functions which know to unpack the type correctly via
any.UnmarshalTo
.I'm unsure if we're making more complicated than it could be as I have very limited knowledge around this. I personally fill like attaching this kind of message to an empty response seems more appealing than utilising GRPC error like this though, but it may not be the best practice.
The issue with this is that it makes us blind in Praefect: we cannot see whether RPCs have failed or not, and this knowledge is essential to us in some places. The only alternative would be to put RPC-specific knowledge into Praefect, and that's definitely something we want to avoid.
So even though this may be complicated, I think it is the only feasible way to go ahead.
Thanks @pks-t Yes, that makes sense. I suppose it shouldn't be a problem once we lay the foundation with this approach.
Can you also confirm if
details
can actually have multiple entries and what do you think we need to do if we have more than one?Can you also confirm if details can actually have multiple entries and what do you think we need to do if we have more than one?
@dskim_gitlab Yes, they can, but I do not plan on making any use of this in Gitaly. With my current design, each RPC would have at most one error detail, where the error detail is then specific to the RPC at hand (e.g.
UserMergeError
). So while it is a theoretical issue for now only, you might want to implement some tooling which iterates through all error details until it finds an expected detail and then returns that expected detail. All the other details should just be ignored.
removed review request for @stanhu
mentioned in issue #3582 (closed)
removed review request for @8bitlife
@pks-t I've just pushed up a draft MR to handle
FailedPrecondition
errors as it seems include merge conflict related errors which I'm mainly interested in as well as a couple other cases. gitlab!67489 (closed)Does that makes sense to you? I suppose we can also include
codes.PermissionDenied
as well if that's known to be safe.I'd like to see if we can update the text I mentioned above to make it clear that there was a conflict as
FailedPrecondition
doesn't seem to be only raised for merge conflicts. I've chosen to go withGitlab::Git::CommitError
as that sounds like it could include merge conflicts. I was thinking about creatingGitlab::Git::ConflictError
, but I don't think we should do that unless gitaly gives us a clear indication that it is a 100% merge conflict.@proglottis Do you have any thoughts on this changes?
I'd like to see if we can update the text I mentioned above to make it clear that there was a conflict as FailedPrecondition doesn't seem to be only raised for merge conflicts.
@dskim_gitlab Eventually, I want to also introduce a
ConflictError
type which we can return from the RPC so that you can get that information. I wanted to focus this MR on the simpler conversion for access checks as a proof of concept for now though. So I'll probably put up another iteration where we just keep the current conflict handling, where the error is then embedded into the normal response. If we agree on the general approach, then I'll iterate and do theConflictError
thing so you can be sure to have a conflict.@pks-t Yes, I agree we should keep this simple so let's add
ConflictError
after this. I'm not sure if we need another iteration though. Should go straightaway introduceConflictError
?I'm not sure if we need another iteration though.
@dskim_gitlab Not sure if we're talking about the same thing when talking about "another iteration". My intent is to land this MR here with only the changed access checks error handling, and then after this has landed I'll go and create another MR which also changes the way we handle conflict errors. Background is mostly that in order to be able to return a proper
ConflictError
, Gitaly would need to figure out what's conflicting in the first place. And we don't yet have that info.Edited by Patrick SteinhardtSo I'll probably put up another iteration where we just keep the current conflict handling, where the error is then embedded into the normal response.
@pks-t I thought by above you meant we'll 2 more MRs to get us to
ConflictError
. Maybe I misunderstoodAnyways, let's get this one properly reviewed by domain experts and move on. I'll work on handing this from the ruby side
Anyways, let's get this one properly reviewed by domain experts and move on. I'll work on handing this from the ruby side
@dskim_gitlab Agreed. I'll try to get the MR into a workable state either today or tomorrow, which will require some additional changes on Praefects side. And then we should be ready to go, I think.
@dskim_gitlab Should be ready now. Praefect failing was really only an invalid test setup: errors get passed through correctly now.
@toon, @samihiltunen Could you both please handle the review of this MR?
mentioned in merge request gitlab!67489 (closed)
mentioned in issue create-stage#12871 (closed)
removed review request for @samihiltunen
added 209 commits
-
78e738fb...f6ebdc9e - 201 commits from branch
master
- be703472 - hook: Fix prereceive returning AllowedError for generic errors
- cfc3c395 - updateref: Generalize `PreReceiveError` according to its usage
- cde1de48 - operations: Allow passing options to `setupOperationsService()`
- 47e3666c - gitlab: Generalize GitLab mock client
- 75bf4832 - operations: Adjust names of merge-related tests
- 9080b136 - helper: Provide error wrappers for `PermissionDenied` codes
- 46755182 - helper: Implement helper to create gRPC errors with details
- 106299cc - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
78e738fb...f6ebdc9e - 201 commits from branch
added 14 commits
-
106299cc...1a92a69a - 6 commits from branch
master
- 01e59fa8 - hook: Fix prereceive returning AllowedError for generic errors
- 5873c46d - updateref: Generalize `PreReceiveError` according to its usage
- 2c700e03 - operations: Allow passing options to `setupOperationsService()`
- 0a2944f2 - gitlab: Generalize GitLab mock client
- 9d21a667 - operations: Adjust names of merge-related tests
- ec0e233c - helper: Provide error wrappers for `PermissionDenied` codes
- db5c2168 - helper: Implement helper to create gRPC errors with details
- dcf4c7b7 - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
106299cc...1a92a69a - 6 commits from branch
changed milestone to %14.3
added 84 commits
-
dcf4c7b7...bea500b3 - 75 commits from branch
master
- d4b1ada0 - hook: Fix prereceive returning AllowedError for generic errors
- 07d9d286 - updateref: Generalize `PreReceiveError` according to its usage
- 04c3b097 - operations: Allow passing options to `setupOperationsService()`
- b45bad8f - gitlab: Generalize GitLab mock client
- a1ea380c - helper: Provide error wrappers for `PermissionDenied` codes
- 800e8ad6 - helper: Implement helper to create gRPC errors with details
- b50282c1 - operations: Adjust names of merge-related tests
- 6ef15409 - operations: Add test exercising UserMergeBranch and access errors
- 499798ce - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
dcf4c7b7...bea500b3 - 75 commits from branch
requested review from @toon and @samihiltunen
added 23 commits
-
499798ce...a0d550e5 - 14 commits from branch
master
- 5b9c0721 - hook: Fix prereceive returning AllowedError for generic errors
- 504b4199 - updateref: Generalize `PreReceiveError` according to its usage
- f6b94dc8 - operations: Allow passing options to `setupOperationsService()`
- b393c55d - gitlab: Generalize GitLab mock client
- 0892f793 - helper: Provide error wrappers for `PermissionDenied` codes
- 618f89a0 - helper: Implement helper to create gRPC errors with details
- 5d42b16d - operations: Adjust names of merge-related tests
- ed2df874 - operations: Add test exercising UserMergeBranch and access errors
- db627667 - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
499798ce...a0d550e5 - 14 commits from branch
- Resolved by Patrick Steinhardt
added 13 commits
-
db627667...32b44d3c - 4 commits from branch
master
- d1b4a1c3 - hook: Fix prereceive returning AllowedError for generic errors
- 448ca89a - updateref: Generalize `PreReceiveError` according to its usage
- 5cc68021 - operations: Allow passing options to `setupOperationsService()`
- eec99771 - gitlab: Generalize GitLab mock client
- a2387d25 - helper: Provide error wrappers for `PermissionDenied` codes
- a9a14137 - helper: Implement helper to create gRPC errors with details
- 99b5794a - operations: Adjust names of merge-related tests
- 10b27473 - operations: Add test exercising UserMergeBranch and access errors
- 125d2baf - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
db627667...32b44d3c - 4 commits from branch
removed review request for @toon
added 28 commits
-
125d2baf...dec431ea - 19 commits from branch
master
- 773668f5 - hook: Fix prereceive returning AllowedError for generic errors
- c6821337 - updateref: Generalize `PreReceiveError` according to its usage
- 0b2a982c - operations: Allow passing options to `setupOperationsService()`
- f34bed05 - gitlab: Generalize GitLab mock client
- 7fee3c93 - helper: Provide error wrappers for `PermissionDenied` codes
- be0671b8 - helper: Implement helper to create gRPC errors with details
- f4c36e35 - operations: Adjust names of merge-related tests
- 2afa4ccd - operations: Add test exercising UserMergeBranch and access errors
- e542a7d8 - operations: Implement rich errors for UserMergeBranch access checks
Toggle commit list-
125d2baf...dec431ea - 19 commits from branch
enabled an automatic merge when the pipeline for 535a5e57 succeeds
mentioned in commit c8311c7d
added workflowstaging label and removed workflowready for review label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
mentioned in issue gitlab#336961 (closed)
mentioned in issue gitlab#339728 (closed)
mentioned in issue gitlab#339727 (closed)
mentioned in merge request !3851 (merged)
mentioned in merge request gitlab!70357 (merged)
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label
mentioned in issue #3845
added workflowstaging-ref label and removed workflowproduction label
mentioned in issue gitlab#342122 (closed)
mentioned in merge request !4120 (merged)
Due to an issue the workflowstaging-ref label was incorrectly applied to this merge request. Re-setting back to workflowproduction using
https://gitlab.com/gitlab-org/release-tools/-/merge_requests/1620
added workflowproduction label and removed workflowstaging-ref label
mentioned in issue gitlab#321358
mentioned in issue gitlab-org/quality/triage-reports#20622 (closed)
mentioned in issue gitlab-org/quality/triage-reports#20986 (closed)
mentioned in issue gitlab-org/quality/triage-reports#21551 (closed)
mentioned in issue gitlab-org/quality/triage-reports#22011