Giter VIP home page Giter VIP logo

Comments (5)

abernix avatar abernix commented on August 20, 2024 1

Any updates to provide here, @mostafao-spotify? Could you add any available answers to the above, including the Router version?

from router.

SimonSapin avatar SimonSapin commented on August 20, 2024 1

Regarding the bridge: validation of incoming queries has moved to Rust but JavaScript is still used for some other components including query planning. We’re porting them to Rust one by one.

Thank you for the schema, with it I managed to the produced the error on the latest dev branch of the Router. However this still needs more investigation. Here is what I have so far:

I composed the schema above into a supergraph schema with Rover and reduced the operation slightly:

supergraph.graphql
schema
  @link(url: "https://specs.apollo.dev/link/v1.0")
  @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION)
{
  query: Query
}

directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION

directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE

directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION

directive @join__graph(name: String!, url: String!) on ENUM_VALUE

directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE

directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR

directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION

directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA

type ActivationItemData
  @join__type(graph: A)
{
  activated: Boolean!
}

input Field1Input
  @join__type(graph: A)
{
  someInputVar1: String!
}

input Field2Input
  @join__type(graph: A)
{
  someInputVar2: String!
  sectionItemsOffset: Int
  sectionItemsLimit: Int
}

scalar join__DirectiveArguments

scalar join__FieldSet

enum join__Graph {
  A @join__graph(name: "A", url: "https://example.com")
}

scalar link__Import

enum link__Purpose {
  """
  `SECURITY` features provide metadata necessary to securely resolve fields.
  """
  SECURITY

  """
  `EXECUTION` features provide metadata necessary for operation execution.
  """
  EXECUTION
}

type MainSection
  @join__type(graph: A)
{
  sectionItems(offset: Int, limit: Int): SectionItemsPage!
}

type PageInfo
  @join__type(graph: A)
{
  offset: Int!
  limit: Int!
}

type Query
  @join__type(graph: A)
{
  field1(input: Field1Input!): ResponsePayload1!
  field2(input: Field2Input!): ResponsePayload1!
}

type ResponsePayload1
  @join__type(graph: A)
{
  data: [ActivationItemData!]!
  sectionContainer: SectionContainer!
}

type SectionContainer
  @join__type(graph: A)
{
  sections(offset: Int, limit: Int): SectionItemsConnection!
}

type SectionItem
  @join__type(graph: A)
{
  data: ActivationItemData
}

type SectionItemsConnection
  @join__type(graph: A)
{
  items: [MainSection!]!
}

type SectionItemsPage
  @join__type(graph: A)
{
  items: [SectionItem!]!
  pageInfo: PageInfo!
}
operation.graphql
fragment section1 on MainSection {
  sectionItems(offset: $sectionItemsOffset, limit: $sectionItemsLimit) {
    items {
      data {
        ... on ActivationItemData {
          activated
        }
      }
    }
  }
}

query query2($var2: String!, $sectionItemsOffset: Int!, $sectionItemsLimit: Int!) {
  field2(
    input: {someInputVar2: $var2}
  ) {
    __typename
    ... on ResponsePayload1 {
      sectionContainer {
        sections(offset: 0, limit: 20) {
          items {
            ...section1
          }
        }
      }
    }
  }
}

query query1 {
  field1(
    input: {someInputVar1: ""}
  ) {
    __typename
  }
}
❯ curl --request POST \
  --header 'content-type: application/json' \
  --url 'http://127.0.0.1:4000/' \
  --data @<(jq -n --rawfile op ~/tmp/operation.graphql '{query: $op, operationName: "query1"}')
{"errors":[{"message":"Invalid value $sectionItemsOffset for argument \"MainSection.sectionItems(offset:)\" of type Int","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}]}

I happened to have my local Router configured with experimental_query_planner_mode: both for another task which added a log entry showing that the Invalid value $sectionItemsOffset is coming from the JavaScript query planner, after we’ve already validated the origin query. I see JS planner code in FetchGroup.finalizeSelection call validate() on the generated subgraph query as a sanity check. I suspect a bug in the planner makes it generate something invalid. I tried the standalone query planner outside of Router to more easily debug JS code but didn’t manage to reproduce the error there. (I get a query plan that looks correct and no error.)

from router.

SimonSapin avatar SimonSapin commented on August 20, 2024

Hi, thank you for this bug report. It’s expected that the entire document is validated, not just the operation specified in the request’s operationName. Validation rules typically start with “For each operation in document”.

That said, this still looks like a bug. Can you confirm: in your schema, is the expected type for the offset argument of the Mainsection.SectionItems field nullable Int like the error message suggests? A variable usage of type non-null Int! should be compatible to use there.

Interestingly, the phrasing of the error message shows that it comes from JavaScript code whereas Apollo Router switched to Rust-based validation by default in 1.45.0. What version of the Router are you using? To investigate this further we’ll likely need to reproduce the error. Could you provide a schema (minimized or not) where it happens?

from router.

mostafao-spotify avatar mostafao-spotify commented on August 20, 2024

Hi @SimonSapin, thanks a lot for investigating this and sorry for the delayed reply, we're working with the router version 1.48.1 and after updating to 1.49.0 the issue still persists.

The offset and limit field in the schema is indeed nullable as the error message suggests, and this is probably by design as null means something to that service, but I think even if we make the field non-nullable that won't solve the underlying issue of mis-interpreting the value.

Notably, one of our team member found this piece of code here that indicates that there's still a javascript bridge in the latest version of the code which we're using, or is this just a non-relevant comment maybe?

As for the schema setup, it looks something like below, but please take it with a grain of salt, as I've tried to obfuscate some of domain details :)

# Query
type Query {
  field1(input: Field1Input!): ResponsePayload1!
  field2(input: Field2Input!): ResponsePayload1!
}

# input
input Field1Input {
  someInputVar1: String!
}

input Field2Input {
  someInputVar2: String!
  sectionItemsOffset: Int,
  sectionItemsLimit: Int
}

# Types
type ResponsePayload1 {
  data: [ActivationItemData!]!
  sectionContainer: SectionContainer!
}

type ActivationItemData {
  activated: Boolean!
}

type SectionContainer {
  sections(offset: Int, limit: Int): SectionItemsConnection!
}

type SectionItemsConnection {
  items: [MainSection!]!
}

type MainSection {
  sectionItems(offset: Int, limit: Int): SectionItemsPage!
}

type SectionItemsPage {
  items: [SectionItem!]!
  pageInfo: PageInfo!
}

type PageInfo {
  offset: Int!
  limit: Int!
}

type SectionItem {
  data: ActivationItemData
}

from router.

mostafao-spotify avatar mostafao-spotify commented on August 20, 2024

Thank you @SimonSapin for looking into this. It's great news to hear that you're moving away from JS Bridge! 🚀
If I can help with anything else, let me know. 🎈

from router.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.