smallrye-graphql icon indicating copy to clipboard operation
smallrye-graphql copied to clipboard

Handling 301 and 302 redirections?

Open gsmet opened this issue 1 year ago • 6 comments

I would have expected the SmallRye GraphQL client to handle both 301 and 302 (i.e. when a resource is moved).

We have the quickstarts failing with:

	Suppressed: io.smallrye.graphql.client.InvalidResponseException: Unexpected response. Code=301, message="Moved Permanently", body="null"
		at io.smallrye.graphql.client.impl.ResponseReader.readFrom(ResponseReader.java:80)
		at io.smallrye.graphql.client.vertx.dynamic.VertxDynamicGraphQLClient.lambda$executeSingleResultOperationOverHttp$10(VertxDynamicGraphQLClient.java:451)
		at io.smallrye.context.impl.wrappers.SlowContextualFunction.apply(SlowContextualFunction.java:21)
		at io.smallrye.mutiny.operators.uni.UniOnItemTransform$UniOnItemTransformProcessor.onItem(UniOnItemTransform.java:36)
		at io.smallrye.mutiny.operators.uni.builders.UniCreateFromCompletionStage$CompletionStageUniSubscription.forwardResult(UniCreateFromCompletionStage.java:63)
		at io.smallrye.context.impl.wrappers.SlowContextualBiConsumer.accept(SlowContextualBiConsumer.java:21)
		at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
		at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841)
		at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
		at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)

gsmet avatar Dec 16 '24 15:12 gsmet

Not that I have no idea if we should have a 301 here and there might be something else to fix.

See this run: https://github.com/quarkusio/quarkus-quickstarts/actions/runs/12344772645 and look for microprofile-graphql-client-quickstart in the logs.

gsmet avatar Dec 16 '24 15:12 gsmet

Hm, did you try setting the quarkus.smallrye-graphql-client."client-name".max-redirects property?

jmartisk avatar Dec 17 '24 05:12 jmartisk

That is weird though, the vertx documentation says that their HTTP client by default follows redirects. We'll have a look

jmartisk avatar Dec 17 '24 06:12 jmartisk

Hm, I think we have several problems here

  • The external graphql service has applied some significant changes, so the old URL (https://swapi-graphql.netlify.app/.netlify/functions/index) now sends a redirect to https://swapi-graphql.netlify.app/graphql
  • For some reason, the vert.x web client doesn't follow redirects even when it is configured to do so (I checked in the debugger)
  • Even after updating the config to use to the new URL directly, I get a 406 error because the service doesn't like a Accept: application/json;charset=utf-8 header and I have to change it to Accept: application/json only
  • Even after I change the header, I get an erroneous GraphQL response:
{"errors":[{"message":"No entry in local cache for https://swapi.dev/api/films/","locations":[{"line":1,"column":18}],"path":["allFilms"]}],"data":{"allFilms":null}}

I think this is calling to stop depending on a third-party service and find a wholly different solution...

jmartisk avatar Dec 18 '24 09:12 jmartisk

I could, as a quick workaround, update the URL and override the header, but I can't get past the "local cache" error, so it wouldn't work anyway

jmartisk avatar Dec 18 '24 09:12 jmartisk

So maybe we could just ignore that test for now and I would come up with a completely different quickstart...

Not sure exactly how to do it,

  • we could include a full-fledged server-side service too
  • stitch up a server side using wiremock
  • make it run against the microprofile-graphql-quickstart but that means creating a dependency between quickstarts
  • find another public GraphQL service that we can rely on better
  • package https://github.com/graphql/swapi-graphql into a container image and somehow run it along with the client app
  • just wait until they fix the original service..

jmartisk avatar Dec 18 '24 09:12 jmartisk