Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crdb: fix watch error: %!s(<nil>) #1660

Merged
merged 2 commits into from
Nov 27, 2023
Merged

crdb: fix watch error: %!s(<nil>) #1660

merged 2 commits into from
Nov 27, 2023

Conversation

vroldanbet
Copy link
Contributor

I identified some error reports in CRDB-based clusters that showed the following:

{
   "level":"error",
   "protocol":"grpc",
   "grpc.component":"server",
   "grpc.service":"authzed.api.v1.WatchService",
   "grpc.method":"Watch",
   "grpc.method_type":"server_stream",
   "grpc.start_time":"2023-11-27T12:38:35Z",
   "grpc.code":"Internal",
   "grpc.error":"rpc error: code = Internal desc = watch error: %!s(<nil>)",
   "grpc.time_ms":1030,
   "time":"2023-11-27T12:38:36Z",
}

I could only identify one spot where a nil
error could be sent over the errors channel,
which would in turn cause SpiceDB response
to have the gRPC error code "Internal".

If the sql.Rows returns an error that isnt
cancellation, then it's possible that nil values
will be sent over the error channel.

I identified some error reports in CRDB based clusters
that showed the following:

{
   "level":"error",
   "protocol":"grpc",
   "grpc.component":"server",
   "grpc.service":"authzed.api.v1.WatchService",
   "grpc.method":"Watch",
   "grpc.method_type":"server_stream",
   "grpc.start_time":"2023-11-27T12:38:35Z",
   "grpc.code":"Internal",
   "grpc.error":"rpc error: code = Internal desc = watch error: %!s(<nil>)",
   "grpc.time_ms":1030,
   "time":"2023-11-27T12:38:36Z",
}

I could only identify one spot where a nil
error could be sent over the errors channel,
which would in turn cause SpiceDB response
to have gRPC error code "Internal".

If the sql.Rows return an error that isnt
cancellation, then it's possible that nil values
will be sent over the error channel.
@vroldanbet vroldanbet requested a review from a team November 27, 2023 13:13
@vroldanbet vroldanbet self-assigned this Nov 27, 2023
@github-actions github-actions bot added the area/datastore Affects the storage system label Nov 27, 2023
@github-actions github-actions bot added area/dependencies Affects dependencies area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Nov 27, 2023
Copy link
Member

@josephschorr josephschorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@josephschorr josephschorr added this pull request to the merge queue Nov 27, 2023
Merged via the queue into main with commit d479967 Nov 27, 2023
20 checks passed
@josephschorr josephschorr deleted the nil-error-crdb-watch branch November 27, 2023 15:50
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/datastore Affects the storage system area/dependencies Affects dependencies area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants