This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
fix/frontend/graphql: log and add trace events for all non-not-found errors that occur when fetching permission syncs #63302
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
team/product-platform
team/source
ggilmore
force-pushed
the
graphite-ggilmore06-17-feature_internal_database_add_test_to_ensure_that_not_found_errors_fulfiill_errcode.notfound
branch
from
4d5e82c to
c3c0cf5
Compare
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
380a90a to
688cede
Compare
|
|
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
688cede to
054ced3
Compare
ggilmore
force-pushed
the
graphite-ggilmore06-17-feature_internal_database_add_test_to_ensure_that_not_found_errors_fulfiill_errcode.notfound
branch
from
c3c0cf5 to
fd80f0c
Compare
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
054ced3 to
d877433
Compare
ggilmore
force-pushed
the
graphite-ggilmore06-17-feature_internal_database_add_test_to_ensure_that_not_found_errors_fulfiill_errcode.notfound
branch
from
fd80f0c to
68da82c
Compare
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
d877433 to
d0fb084
Compare
ggilmore
force-pushed
the
graphite-ggilmore06-17-feature_internal_database_add_test_to_ensure_that_not_found_errors_fulfiill_errcode.notfound
branch
from
68da82c to
b7026c4
Compare
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
d0fb084 to
782e733
Compare
ggilmore
changed the base branch from
graphite-ggilmore06-17-feature_internal_database_add_test_to_ensure_that_not_found_errors_fulfiill_errcode.notfound
to
graphite-base/63302
…errors that occur when fetching permission syncs
ggilmore
force-pushed
the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
from
782e733 to
79a43a0
Compare
ggilmore
deleted the
graphite-ggilmorefix_frontend_graphql_log_and_add_trace_events_for_all_non-not-found_errors_that_occur_when_fetching_permission_syncs
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes https://linear.app/sourcegraph/issue/SRC-421/customer-reported-an-issue-with-user-permissions-stats
This PR ensures that any non not-found errors that occur when fetching permission sync jobs are logged and added as error events in the trace. Before, these errors were silently swallowed which makes things exceptionally hard to debug.
This PR only changes the monitoring around this query - it doesn't change the underlying behavior.
Note: I spent a while reading up on how to communicate partial errors in our graphql API, but it seems like there isn't a clear consensus.
According to the above, it seems like ideally we should be returning
nullin the graphQL for permission sync jobs that have an error. However, when I tried to do this I begain seeing nil panics all over the place. It's hard for me to know if the underlying library supports this or if we are just holding it wrong. 😥Test plan
Unit tests
Changelog