Repository navigation
turn off async-graphql's blanket error conversion - #2881
Merged
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 44af998 | Previous: a05fce6 | Ratio |
|---|---|---|---|
lotr_graph_subgraph_10pc/num_nodes |
10 ns/iter (± 0) |
4 ns/iter (± 0) |
2.50 |
lotr_graph_subgraph_10pc/num_nodes_cached |
9 ns/iter (± 0) |
4 ns/iter (± 0) |
2.25 |
This comment was automatically generated by workflow using github-action-benchmark.
ljeub-pometry
requested changes
Nov 5, 2026
shivamka1
force-pushed
the
fix/policy-fault-answers
branch
from
November 5, 2026 13:12
cc84a32 to
222d0e4
Compare
async-graphql turned any error that can print itself into a client error at every `?`, so whatever an internal error said reached the caller, with no extension code, and nothing made a resolver decide. With custom-error-conversion on, each error type that reaches a resolver says what the client sees, next to its definition: - PermissionError keeps its code, which a `?` used to drop. - AuthPolicyError answers the generic policy error and logs the detail. - AuthError's access denials carry ACCESS_DENIED. - GQLError passes the graph layer's message through, except an I/O failure, which is logged and answered as an internal server error. - The other local errors pass their message through, as before. GraphError lives in raphtory, so it reaches a resolver through GQLError, which now accepts anything that converts into a GraphError.
The write gates spelled out AuthError's ACCESS_DENIED by hand, the metadata summary mapped AuthPolicyError by hand, and two resolvers turned their errors into bare strings, which also dropped any code. Each is now a plain `?` through the error type's own conversion.
GQLError took any error convertible into GraphError, so an io::Error behind `?` became GQLError::GraphError and reached the client with its own text, which can name server paths. The blanket impl is gone: io errors convert to GQLError::Io, which is logged and answered as "Internal server error", and GraphError plus the other graph-layer errors that relied on the blanket each convert explicitly.
apollo_tracing and custom-error-conversion move from raphtory-graphql's own dependency entry to the workspace one, so every crate in the workspace builds async-graphql the same way.
Resolvers and the permission gates they call returned async_graphql's error, so a GraphError could only reach them through a map_err, and each local error type carried its own conversion to the client error. They now return GQLError, which gains variants for permission, auth, deletion and batch failures, so `?` converts and GQLError's conversion is the only one left; the codes a denial carries are set there. GqlGraphError, five of whose six variants were unused, folds into GQLError, and the two denials that were built by hand are typed: a read-only context is PermissionError::ReadOnlyContext and a grant whose filter cannot be applied is GQLError::AccessFilterMisconfigured. A malformed namespace-listing filter is GQLError::InvalidFilter. raphtory re-exports StorageError beside read_constant_graph_properties, which returns it, so it converts like the other graph-layer errors.
The RDF resolvers returned GraphError or async_graphql's error and reached the client through async-graphql's blanket Display conversion, which custom-error-conversion turns off. They now return GQLError. SparqlError becomes one of its variants, so the codes a SPARQL or SHACL limit carries are set in GQLError's conversion with the others, and an RdfError converts through it whichever resolver raises it. The SHACL shapes-format refusal is typed as SparqlError::ShapesFormat.
shivamka1
force-pushed
the
feat/explicit-gql-errors
branch
from
November 5, 2026 14:16
fd343db to
68d11e2
Compare
ljeub-pometry
approved these changes
Nov 5, 2026
shivamka1
force-pushed
the
feat/explicit-gql-errors
branch
from
November 5, 2026 14:51
e42f501 to
0ac353d
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacked on #2880; retarget to
db_v4once it merges. v4-storage: when its Raphtory pin moves past this PR, the same commit must addapollo_tracingandcustom-error-conversionto its workspaceasync-graphqlentry, because v4-storage buildsraphtory-graphqlas its own member.async-graphql turns any error that can print itself into a client error at every
?. Whatever an internal error says reaches the caller, without an extension code, and nothing makes a resolver decide. This enablescustom-error-conversionand routes every resolver error through one type,GQLError, whose conversion is the only place a client error is built.PermissionErrorACCESS_DENIED,GRAPH_NOT_FOUND,AUTH_ERROR)AuthPolicyErrorAuthErrorACCESS_DENIEDrdf)RESULTS_TOO_LARGEorTIMEOUTInternal server error; detail loggedGraphErrorand the errors it wraps,PathValidationError,InsertionError,DeletionError,ParquetPathError,BatchFailures, a malformed listing filterResolvers and the permission gates return
GQLError, so?converts without amap_err. Each error that reaches a resolver throughGraphErrorhas its ownFromimpl. A blanket impl would send anio::ErrorthroughGraphErrorand show its text.GqlGraphErroris gone, and raphtory re-exportsStorageErrorbesideread_constant_graph_properties, which returns it. The schema is unchanged.Decisions for review:
GraphErrorpasses through unchanged. Some variants wrap storage or I/O errors and may name server paths. Should those be logged and answered generically too?timeindex.rs,property.rs) and history formatting still build errors from strings, because those describe the client's own input or data.GQLError::InvalidFilter, while theGqlFilterconversions still useGraphError::InvalidGqlFilter(moveGraphError::InvalidGqlFilterout of raphtory core #2886).