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

After upgrading to DGS10.0.0 the response headers doesn't have custom headers set #2101

Closed
dokkaraman opened this issue Jan 3, 2025 Discussed in #2100 · 8 comments
Closed
Labels
bug Something isn't working

Comments

@dokkaraman
Copy link

dokkaraman commented Jan 3, 2025

Discussed in #2100

Originally posted by dokkaraman January 3, 2025
After upgrading to DGS10.0.0 the headers (Cache-Control) set via SimplePerformantInstrumentation no longer get set in the final response of a subgraph
This works till version 9.2.1

@paulbakker
Copy link
Collaborator

Can you provide some details about what you're doing in the SimplePerformantInstrumentation implementation?

@dokkaraman
Copy link
Author

We are adding Cache Control header as part of custom instrumentation which extends SimplePerformantInstrumentation
The value of cache control is variable which gets determined in this class.
This used to work till version 9.1.2 of DGS and after upgrade it started failing

@paulbakker
Copy link
Collaborator

SimplePerformantInstrumentation doesn't have access to request/response without using some other context mechanism, so can you share exactly how you're doing this? That might explain why this changed hopefully.

@dokkaraman
Copy link
Author

dokkaraman commented Jan 7, 2025

We use spring framework and here is boiler plate code

import com.netflix.graphql.dgs.DgsExecutionResult;
import graphql.ExecutionResult;
import graphql.GraphQLContext;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.execution.instrumentation.SimplePerformantInstrumentation;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;

@Component
@Slf4j
public class ResponseHeaderInstrument extends SimplePerformantInstrumentation {

    @Value("${entity.cacheMaxAge:max-age=60}")
    private String cacheMaxAge;


    @Override
    public CompletableFuture<ExecutionResult> instrumentExecutionResult(ExecutionResult executionResult,
                                                                        InstrumentationExecutionParameters parameters,
                                                                        InstrumentationState state) {
        HttpHeaders responseHeaders = new HttpHeaders();
        List<String> subgraphCacheControls = new ArrayList<>();

        try {
            if (parameters != null) {
                var gqlContext = parameters.getGraphQLContext();
                ....
                //Logic to add subgraphCacheControls list
                .....
            }
        } catch (Exception e) {
            log.warn("Exception while processing gqlContext", e);
        }

        if (!subgraphCacheControls.isEmpty()) {
            subgraphCacheControls.forEach(cacheControl -> responseHeaders.add("cache-control", cacheControl));
        } else {
            responseHeaders.add("cache-control", cacheMaxAge);
        }

        return super.instrumentExecutionResult(
                DgsExecutionResult.builder().executionResult(executionResult).headers(responseHeaders).build(),
                parameters,
                state
        );
    }
    

@dokkaraman
Copy link
Author

If I change the above code to below it works

import graphql.GraphQLContext;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.graphql.server.WebGraphQlInterceptor;
import org.springframework.graphql.server.WebGraphQlRequest;
import org.springframework.graphql.server.WebGraphQlResponse;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;


@Component
@Slf4j
class ResponseHeaderInstrument implements WebGraphQlInterceptor {
    @Value("${entity.cacheMaxAge:max-age=60}")
    private String cacheMaxAge;


    @Override
    public Mono<WebGraphQlResponse> intercept(WebGraphQlRequest request, Chain chain) {
        return chain.next(request).doOnNext(response -> {
            response.getExecutionInput().getGraphQLContext();
            GraphQLContext
                gqlContext = (response != null && response.getExecutionInput() != null) ? response.getExecutionInput().getGraphQLContext() : null;
            HttpHeaders responseHeaders = getResponseHeadersFromContext(gqlContext, cacheMaxAge);
            log.debug("Response headers: {}", responseHeaders);
                response.getResponseHeaders().addAll(responseHeaders);
        });
    }

    private HttpHeaders getResponseHeadersFromContext(GraphQLContext gqlContext, String cacheMaxAge) {
        HttpHeaders responseHeaders = new HttpHeaders();
        List<String> subgraphCacheControls = new ArrayList<>();

        try {
            if (gqlContext != null) {
                ....
                //Logic to add subgraphCacheControls list
                .....
            }
        } catch (Exception e) {
            log.warn("Exception while processing gqlContext", e);
        }

        if (!subgraphCacheControls.isEmpty()) {
            subgraphCacheControls.forEach(cacheControl -> responseHeaders.add("cache-control", cacheControl));
        } else {
            responseHeaders.add("cache-control", cacheMaxAge);
        }

        return responseHeaders;
    }
   

@paulbakker
Copy link
Collaborator

Thanks for the extra information. It looks like what changed is setting headers on DgsExecutionResult.
Using WebGraphQlInterceptor is probably a better mechanism, because it better separates GraphQL execution from transport related concerns, but since this is changed behavior we should look into it further.

@paulbakker paulbakker added the bug Something isn't working label Jan 7, 2025
@paulbakker
Copy link
Collaborator

I have looked into this further and created a fix here: #2104

@paulbakker
Copy link
Collaborator

Fixed by #2104

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

No branches or pull requests

2 participants