Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Reactive Federated Graph errors out when returning a Mono in a DGSEntityFetcher #876

Closed
BCantos17 opened this issue Feb 11, 2022 · 11 comments · Fixed by #903
Closed
Labels
bug Something isn't working

Comments

@BCantos17
Copy link
Contributor

BCantos17 commented Feb 11, 2022

Expected behavior

Given a reactive federated graph, framework should return requested fields in schema without issues

Actual behavior

Returns error message stating no definition found for ...MonoJust.

Note: I tested this out without using Mono and it works fine

{
  "errors": [
    {
      "message": "Can't resolve '/_entities[0]'. Abstract type '_Entity' must resolve to an Object type at runtime for field '[_Entity]._entities'. Could not determine the exact type of '_Entity'",
      "extensions": {
        "code": "DOWNSTREAM_SERVICE_ERROR",
        "serviceName": "review",
        "classification": "DataFetchingException",
        "exception": {
          "message": "Can't resolve '/_entities[0]'. Abstract type '_Entity' must resolve to an Object type at runtime for field '[_Entity]._entities'. Could not determine the exact type of '_Entity'",
          "path": [
            "_entities",
            0
          ],
          "stacktrace": [
            "GraphQLError: Can't resolve '/_entities[0]'. Abstract type '_Entity' must resolve to an Object type at runtime for field '[_Entity]._entities'. Could not determine the exact type of '_Entity'", ... other message with personal data

          ]
        }
      }
    }
  ],
  "data": {
   ... data
  }
}
2022-02-11 14:28:26.189  WARN 89198 --- [ctor-http-nio-2] c.n.g.d.f.DefaultDgsFederationResolver   : No type definition found for reactor.core.publisher.MonoJust. You probably need to provide either a type mapping,or override DefaultDgsFederationResolver.typeResolver().Alternatively make sure the type name in the schema and your Java model match
2022-02-11 14:28:26.191  WARN 89198 --- [ctor-http-nio-2] n.graphql.execution.ExecutionStrategy    : Can't resolve '/_entities[9]'. Abstract type '_Entity' must resolve to an Object type at runtime for field '[_Entity]._entities'. Could not determine the exact type of '_Entity'

graphql.execution.UnresolvedTypeException: Could not determine the exact type of '_Entity'

Steps to reproduce

Define a federated schema

type Query {
  shows(titleFilter: String): [Show]
}

type Show @key(fields: "id") {
  id: ID
  title: String
  releaseYear: Int
}

Extend the federated schema

type Show @key(fields: "id") @extends {
  id: ID @external
  reviews: [Review]
}

type Review {
  starRating: Int
}

Define entity fetcher in extended service with returning an object with Mono.just(...)

@DgsEntityFetcher(name = "Show")
fun show(values: Map<String, Any>): Mono<Show> {
        return Mono.just(Show( values.get("id") as String, null))
}

// Don't think it makes it here but including it just in case
@DgsData(parentType = "Show", field = "reviews" )
fun reviewFetcher(dfs: DgsDataFetchingEnvironment): Mono<Review> {
    val show = dfe.getSource<Show>()
    val loader = dfe.getDataLoader<String, Review>(reviews)
    val future = loader.load(show.id)
    return Mono.fromCompletionStage(future)
}
@BCantos17 BCantos17 added the bug Something isn't working label Feb 11, 2022
@gdagitrep
Copy link

Any updates on this, I am facing similar issue? DgsQuery works fine with Mono/Flux, but DgsEntityFetcher is troublesome.

@gdagitrep
Copy link

Found similar issue #376, #389 , per which support was added in graphql-dgs-webflux-starter:4.2.0. But I am using 4.9.20, still facing it.

@BCantos17 any workarounds found?

@sshevlyagin
Copy link

sshevlyagin commented Feb 22, 2022

We're running into this as well :/

Tagging @paulbakker I didn't see explicit tests for DgsEntityFetcher in #389 does it require a similar but different fix?

@BCantos17
Copy link
Contributor Author

Found similar issue #376, #389 , per which support was added in graphql-dgs-webflux-starter:4.2.0. But I am using 4.9.20, still facing it.

@BCantos17 any workarounds found?

I have not found a work around using project reactor. Currently the plan is to keep this service as a non-reactive app until this is fixed

@gdagitrep
Copy link

@BCantos17

Found the following works: remove the mono only from the parent DgsEntityFetcher and keep the Mono in the actual field resolver. Using it as a work around for now until the bug is fixed.

@DgsEntityFetcher(name = "Show")
fun show(values: Map<String, Any>): Show {
        return new Show( values.get("id") as String, null)
}

@DgsData(parentType = "Show", field = "reviews" )
fun reviewFetcher(dfs: DgsDataFetchingEnvironment): Mono<Review> {
    val show = dfe.getSource<Show>()
    val loader = dfe.getDataLoader<String, Review>(reviews)
    val future = loader.load(show.id)
    return Mono.fromCompletionStage(future)
}

@srinivasankavitha
Copy link
Contributor

Thanks for reporting this issue. We will look into fixing this in the next release or so.

@bogdanbrindusan
Copy link

it's an issue for us as well.
Continuing to use DGS would mean switching the entire service to non-reactive which is not feasible.

@jord1e
Copy link
Contributor

jord1e commented Mar 3, 2022

it's an issue for us as well. Continuing to use DGS would mean switching the entire service to non-reactive which is not feasible.

This is total nonsense

Change your return type from Mono<T> to CompletableFuture<T> and call Mono#toFuture. Only do this on methods annotated with @DgsEntityFetcher:

CompletableFuture<String> cf = Mono.just("").toFuture();

Maybe this does not work, then use Mono#block until the issue is resolved

@bogdanbrindusan
Copy link

@jord1e alright, a bit harsh, but fair. I admit I haven't looked deeply into why isn't it working before posting.

But thanks a lot for the suggestion; tested it and it works.

@srinivasankavitha
Copy link
Contributor

@jord1e - Thanks so much for responding and suggesting a workaround. We will address this soon, we've been a bit busy with our current priorities and haven't gotten around to fixing this. Will do so in an upcoming release.

@jord1e
Copy link
Contributor

jord1e commented Mar 3, 2022

Sorry for the rude reaction @bogdanbrindusan

I had some free time, I fixed the issue @srinivasankavitha (consider it my 2024 Netflix internship application 😜)
#903 (fix) and Netflix/dgs#84 (docs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants