apollographql / router

A configurable, high-performance routing runtime for Apollo Federation 🚀
https://www.apollographql.com/docs/router/
Other
793 stars 266 forks source link

Document how to propagate extensions from subgraph responses #2504

Open lleadbet opened 1 year ago

lleadbet commented 1 year ago

Describe the bug It appears the Router is currently stripping the root level extensions field from the response to clients, even though subgraphs are responding with them in the request.

To Reproduce

Given this dummy subgraph:

const express = require('express')
const app = express()
const port = 4001

let resp = {
    "errors":[
        {
            "code": "TEST_CODE",
            "message":"Code isn't a defined root field value, but okay according to the specification...",
            extensions: 
                {
                    test: 1,
                }

        }
    ],
    "data":{
        "id":null
    },
    "extensions":{
        "2": "hullo",
        test: 1234,
    }
}

app.post('/', (req, res) => {
    res.setHeader('content-type', 'application/json')
    res.send(JSON.stringify(resp))
})

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`)
})

and this supergraph:

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

directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) 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) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR

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

scalar join__FieldSet

enum join__Graph {
  _1_ @join__graph(name: "1", url: "http://localhost:4001")
  _2_ @join__graph(name: "2", url: "http://localhost:4002")
}

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 Query
  @join__type(graph: _1_)
  @join__type(graph: _2_)
{
  getError: String @join__field(graph: _1_)
  getUser: User @join__field(graph: _2_)
}

type User
  @join__type(graph: _2_, key: "id")
{
  id: ID!
  name: String!
}

If you query the server directly, you get:

{
  "errors": [
    {
      "code": "TEST_CODE",
      "message": "Code isn't a defined root field value, but okay according to the specification...",
      "extensions": {
        "test": 1
      }
    }
  ],
  "data": {
    "id": null
  },
  "extensions": {
    "2": "hullo",
    "test": 1234
  }
}

However the router returns:

{
  "data": {
    "getError": null
  },
  "errors": [
    {
      "message": "Code isn't a defined root field value, but okay according to the specification...",
      "extensions": {
        "test": 1
      }
    }
  ]
}

It is possible to work around this using Rhai currently, however:

fn supergraph_service(service) {
  let f = |response| {
    if response.context["extensions"]?.len > 0 {
      for extension in response.context["extensions"] {
        response.body.extensions += extension;
      }
    }
  };

  service.map_response(f);
}

fn subgraph_service(service, subgraph) {
    let f = |response| {
      if response.body.extensions != () {
        if response.context["extensions"] == () {
          response.context.extensions = [];
        }

        response.context.extensions += response.body.extensions
      }
    };

    service.map_response(f);
}

Expected behavior This feels as though it should be supported natively, either as a config option or by default.

bnjjj commented 1 year ago

Hmm that's because it's not really part of the specs https://spec.graphql.org/October2021/#note-5c13b we should investigate what we want to accept or move into extensions

lleadbet commented 1 year ago

Fair, although this is something that AS3/4 both can do out of the box. The above Rhai might be the only solution, but we should document it as such.

smyrick commented 1 year ago

It is part of the spec Oct2021 for both response.extensions and response.errors[].extensions.

smyrick commented 1 year ago

@lleadbet is your subgraph response invalid in this example though? Shouldn't it be for the query { getUser { id } }

{
  "errors": [
    {
      "code": "TEST_CODE",
      "message": "Code isn't a defined root field value, but okay according to the specification...",
      "extensions": {
        "test": 1
      }
    }
  ],
  "data": {
    "getUser": {
      "id": null
    }
  },
  "extensions": {
    "2": "hullo",
    "test": 1234
  }
}
lleadbet commented 1 year ago

Yeah, I had updated it at some point to return:

{
    "data": {
        "getError": "null"
    },
    "extensions": {
        "2": "hullo",
        test: 1234,
    }
}

which still exhibits the same issue here (missing response.extensions)

abernix commented 1 year ago

@bnjjj As noted above, response.extensions is indeed a spec-valid thing — we even use it in the Router to return query plans when running in --dev mode. I don't think that we necessarily want to move anything new into extensions, but it is surface area that users should be able to use if they want.

Fwiw, I can't tell if you're suggesting this was the case or not, but I didn't/don't think that the Gateway ever established or supported any method for exposing extensions returned from subgraphs — mostly because that gets very squarely into "stomping on each other's extensions" territory without thoughtfully ordered rules on how those should be merged.

chandrikas commented 1 year ago

We feel this can be done via Rhai. Let's validate that and document it.