Skip to content

Correct Gitaly stream handling

Mikhail Mazurskiy requested to merge ash2k/better-gitaly-error-handling into master

Closes #88 (closed).

Bug 1

Mystery solved: Gitaly returns an empty response message when file is not found, which we don't handle properly. A better API would be to return a proper gRPC NotFound status. Opened an MR gitlab-org/gitaly!3078 (merged)

We were printing expected BLOB got COMMIT because we are inspecting the Type of the TreeEntryResponse and for an empty response it is COMMIT because it's value is 0.

Old agentk logs (note how there was no way to know what's going on from the client side. Well, we needed and still need gitlab-org/gitlab#277323 (closed)):

<nothing!>

New agentk logs:

{"level":"warn","time":"2021-01-30T16:42:53.822+1100","msg":"GetConfiguration.Recv failed","error":"rpc error: code = FailedPrecondition desc = Config: agent configuration file: FileNotFound: TreeEntry.Recv: file/directory/ref not found: .gitlab/agents/new1/config.yaml"}

Old kas logs:

{"level":"error","time":"2021-01-30T16:47:27.992+1100","msg":"Config: failed to fetch","correlation_id":"01EX8X3TSKB7K733SQ43DKXBM4","agent_id":5,"project_id":"root/network-policy-demo","error":"fetch agent configuration: TreeEntry: expected BLOB got COMMIT"}

New kas logs (note how we log on info rather than on error which is correct for user errors):

{"level":"info","time":"2021-01-30T16:48:38.340+1100","msg":"Config: failed to fetch","correlation_id":"01EX8XC31FNTH63805SCAAA33J","agent_id":5,"project_id":"root/network-policy-demo","error":"agent configuration file: FileNotFound: TreeEntry.Recv: file/directory/ref not found: .gitlab/agents/new1/config.yaml"}

Bug 2

Gitaly sets the type of response in the first message only, then clears all fields. This was not handled correctly.

Edited by Mikhail Mazurskiy

Merge request reports