apollo-rs
apollo-rs copied to clipboard
Published
20 hours ago •
apollographql
apollographql
graceful handling of syntax and semantic errors: `apollo-compiler` panics on syntax errors
if you place characters after a [Type], like [Type]oops, apollo-compiler will panic.
example valid schema:
type Person {
id: ID!
name: String
appearedIn: [Film]
directed: [Film]
}
type Film {
id: ID!
title: String
actors: [Person]
director: Person
}
type Query {
person(id: ID!): Person
people: [Person]
film(id: ID!): Film!
films: [Film]
}
example panic schema:
type Person {
id: ID!
name: String
appearedIn: [Film]s
directed: [Film]
}
type Film {
id: ID!
title: String
actors: [Person]
director: Person
}
type Query {
person(id: ID!): Person
people: [Person]
film(id: ID!): Film!
films: [Film]
}
abbreviated panic
$ rover schema lint --schema ./schema.out
Validating that --schema ./schema.out conforms to the GraphQL specification.
thread 'main' panicked at 'Field must have a type', /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:963:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
panic with full backtrace
$ rover schema lint --schema ./schema.out
Validating that --schema ./schema.out conforms to the GraphQL specification.
thread 'main' panicked at 'Field must have a type', /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:963:28
stack backtrace:
0: rust_begin_unwind
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:584:5
1: core::panicking::panic_fmt
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:143:14
2: core::panicking::panic_display
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:73:5
3: core::panicking::panic_str
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:56:5
4: core::option::expect_failed
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/option.rs:1852:5
5: core::option::Option<T>::expect
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/option.rs:715:21
6: apollo_compiler::queries::database::field_definition
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:963:17
7: core::ops::function::FnMut::call_mut
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:150:5
8: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:280:13
9: core::option::Option<T>::map
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/option.rs:906:29
10: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/iter/adapters/map.rs:103:9
11: alloc::vec::Vec<T,A>::extend_desugared
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/mod.rs:2649:35
12: <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/spec_extend.rs:18:9
13: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/spec_from_iter_nested.rs:43:9
14: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/spec_from_iter.rs:33:9
15: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/mod.rs:2552:9
16: core::iter::traits::iterator::Iterator::collect
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/iter/traits/iterator.rs:1778:9
17: apollo_compiler::queries::database::fields_definition
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:949:48
18: apollo_compiler::queries::database::object_type_definition
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:709:29
19: apollo_compiler::queries::database::object_types::{{closure}}
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:442:22
20: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:269:13
21: <core::slice::iter::Iter<T> as core::iter::traits::iterator::Iterator>::find_map
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/slice/iter/macros.rs:276:38
22: <core::iter::adapters::filter_map::FilterMap<I,F> as core::iter::traits::iterator::Iterator>::next
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/iter/adapters/filter_map.rs:61:9
23: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/spec_from_iter_nested.rs:26:32
24: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/spec_from_iter.rs:33:9
25: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/vec/mod.rs:2552:9
26: core::iter::traits::iterator::Iterator::collect
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/iter/traits/iterator.rs:1778:9
27: apollo_compiler::queries::database::object_types
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:437:19
28: <apollo_compiler::queries::database::ObjectTypesQuery as salsa::plumbing::QueryFunction>::execute
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:50:8
29: salsa::derived::slot::Slot<Q,MP>::read_upgrade::{{closure}}
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:218:13
30: salsa::runtime::Runtime::execute_query_implementation
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/runtime.rs:330:21
31: salsa::derived::slot::Slot<Q,MP>::read_upgrade
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:215:26
32: salsa::derived::slot::Slot<Q,MP>::read
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:148:9
33: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived.rs:170:13
34: salsa::QueryTable<Q>::try_get
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/lib.rs:494:9
35: salsa::QueryTable<Q>::get
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/lib.rs:490:9
36: <DB as apollo_compiler::queries::database::SourceDatabase>::object_types::__shim
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:27:1
37: <DB as apollo_compiler::queries::database::SourceDatabase>::object_types
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:27:1
38: apollo_compiler::queries::database::add_object_type_id_to_schema
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:899:55
39: apollo_compiler::queries::database::schema
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:428:21
40: <apollo_compiler::queries::database::SchemaQuery as salsa::plumbing::QueryFunction>::execute
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:48:8
41: salsa::derived::slot::Slot<Q,MP>::read_upgrade::{{closure}}
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:218:13
42: salsa::runtime::Runtime::execute_query_implementation
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/runtime.rs:330:21
43: salsa::derived::slot::Slot<Q,MP>::read_upgrade
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:215:26
44: salsa::derived::slot::Slot<Q,MP>::read
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived/slot.rs:148:9
45: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/derived.rs:170:13
46: salsa::QueryTable<Q>::try_get
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/lib.rs:494:9
47: salsa::QueryTable<Q>::get
at /home/a/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.16.1/src/lib.rs:490:9
48: <DB as apollo_compiler::queries::database::SourceDatabase>::schema::__shim
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:27:1
49: <DB as apollo_compiler::queries::database::SourceDatabase>::schema
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/queries/database.rs:27:1
50: apollo_compiler::validation::schema::check
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/validation/schema.rs:13:8
51: apollo_compiler::validation::Validator::validate
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/validation/mod.rs:40:33
52: apollo_compiler::ApolloCompiler::validate
at /home/a/.cargo/git/checkouts/apollo-rs-c2f4fdeb51021351/2008021/crates/apollo-compiler/src/lib.rs:37:9
53: rover::command::schema::lint::Lint::run
at ./src/command/schema/lint.rs:28:22
54: rover::command::schema::Schema::run
at ./src/command/schema/mod.rs:24:39
55: rover::cli::Rover::execute_command
at ./src/cli.rs:189:41
56: rover::cli::Rover::run
at ./src/cli.rs:127:34
57: rover::main::{{closure}}
at ./src/bin/rover.rs:15:5
58: rover::main
at ./src/bin/rover.rs:5:1
59: core::ops::function::FnOnce::call_once
at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace
I was gonna open an issue but it looks the same is this.
Test case: add crates/apollo-compiler/test_data/diagnostics/0050_unnamed_object_type.graphql
type {
cat: String
}
cargo test:
thread 'tests::compiler_tests' panicked at 'Field must have a name', crates/apollo-compiler/src/database/hir_db.rs:1092:24
In general, any use of Option::expect or similar should probably be replaced by something that emits a diagnostic and continues with some kind of fallback (such as skipping ill-formed definitions in the HIR)
Yes! I think I'll be working on this next as I'm already having to write more .expect()s changing some of our From impls to TryFrom, and it feels bad (and it is bad for untrusted user input 😇 ).
I think our first step will be to outright skip things in the HIR. If the HIR can't be generated fully, it should be because there was a parser error, which is already reported.
ugh yea this is handled really rather terribly atm (i only say this because i am responsible >.<). would be so very good to fix this!!
there are a few overarching considerations when uniting the two types of errors here:
- syntax errors are stored separately from tokens in the ast, when creating the HIR node, we should try to correlate the fact the ast node is missing because of a specific syntax error, or just output the syntax error as part of diagnostics
- how do we provide as much as diagnostics as possible? let's say a name for a fragment definition is missing, and there are also some semantic errors in that fragment definition (e.g. field doesn't exist on the interface it's referring to). do we want to scrap the existence of the whole fragment definition node and therefore not do any validation on it until the syntax error is fixed? or do we try to continue with semantic validation of the parts that exist?
