concurrent map writes in the module example

[flymedllva]

Component(s)

router

What happened?

Description

When using the module from the example, sometimes the error fatal error: concurrent map writes occurs

This happens if you set a debug breakpoint on ctx.ResponseWriter().Header().Set("myHeader", ctx.GetString("myValue")), restart the router and send a request

Steps to Reproduce

Run router with module use 0.54.0 tag

Expected Result

No error

Actual Result

ctx.ResponseWriter().Header().Set("myHeader", ctx.GetString("myValue")) -> fatal error: concurrent map writes

Component version

135a54f

Environment information

Environment

OS: macOS 14.2.1 (23C71) M1
Package Manager: wgc downloaded from npm
Compiler(if manually compiled): go1.21.4

Router configuration

version: "1"

# General router options
graph:
  name: "production"
  token: ""

log_level: "info"
listen_addr: "localhost:3002"
playground_enabled: true
introspection_enabled: true
json_log: true
shutdown_delay: 15s
grace_period: 20s
poll_interval: 10s
health_check_path: "/health"
readiness_check_path: "/health/ready"
liveness_check_path: "/health/live"
router_config_path: "config.json"

cors:
  allow_origins: ["*"]
  allow_methods:
    - HEAD
    - GET
    - POST
  allow_headers:
    - Origin
    - Content-Length
    - Content-Type
  allow_credentials: true
  max_age_minutes: 5m

# Config for custom modules   
# See "https://cosmo-docs.wundergraph.com/router/custom-modules" for more information   
modules:
  myModule:
    # Arbitrary values, unmarshalled by the module
    value: 1

Router execution config

No response

Log output

fatal error: concurrent map writes

goroutine 254 [running]:
net/textproto.MIMEHeader.Set(0x14001a40150, {0x101cea3a9, 0x8}, {0x101ce8ce8, 0x7})
        /opt/homebrew/opt/go/libexec/src/net/textproto/header.go:22 +0xbc
net/http.Header.Set(0x14001a40150, {0x101cea3a9, 0x8}, {0x101ce8ce8, 0x7})
        /opt/homebrew/opt/go/libexec/src/net/http/header.go:40 +0x34
github.com/wundergraph/cosmo/router/cmd/custom/module.(*MyModule).OnOriginResponse(0x14000420a60, 0x14002e1a360, {0x102362a90, 0x140009de900})
        /Users/dgridnev/projects/cosmo/router/cmd/custom/module/module.go:57 +0xa8
github.com/wundergraph/cosmo/router/core.(*CustomTransport).RoundTrip(0x1400027c960, 0x1400023cb00)
        /Users/dgridnev/projects/cosmo/router/core/transport.go:130 +0x318
net/http.send(0x1400023c800, {0x10234bbc0, 0x1400027c960}, {0xc16503202768d630, 0x73df475442, 0x102d3a380})
        /opt/homebrew/opt/go/libexec/src/net/http/client.go:260 +0x3a0
net/http.(*Client).send(0x140001a2f00, 0x1400023c800, {0xc16503202768d630, 0x73df475442, 0x102d3a380})
        /opt/homebrew/opt/go/libexec/src/net/http/client.go:181 +0xe0
net/http.(*Client).do(0x140001a2f00, 0x1400023c800)
        /opt/homebrew/opt/go/libexec/src/net/http/client.go:724 +0xdd0
net/http.(*Client).Do(0x140001a2f00, 0x1400023c800)
        /opt/homebrew/opt/go/libexec/src/net/http/client.go:590 +0x3c
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/httpclient.Do(0x140001a2f00, {0x102359940, 0x14001065800}, {0x14001778d80, 0x169, 0x169}, {0x10234bc00, 0x14001064c90})
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/datasource/httpclient/nethttpclient.go:127 +0x518
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/graphql_datasource.(*Source).Load(0x1400011a2f8, {0x102359940, 0x14001065800}, {0x14001778d80, 0x169, 0x169}, {0x10234bc00, 0x14001064c90})
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/datasource/graphql_datasource/graphql_datasource.go:1767 +0x8c
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve.(*Loader).executeSourceLoad(0x1400357fb80, {0x102359978, 0x14003ff44b0}, {0x10234ea00, 0x1400011a2f8}, {0x140033f8000, 0x169, 0x280}, 0x14001064c90, 0x14000482580)
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/resolve/loader.go:898 +0x138c
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve.(*Loader).loadEntityFetch(0x1400357fb80, {0x102359978, 0x14003ff44b0}, 0x14000219a40, {0x140014505c0, 0x1, 0x1}, 0x14002e92160)
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/resolve/loader.go:605 +0x878
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve.(*Loader).loadFetch(0x1400357fb80, {0x102359978, 0x14003ff44b0}, {0x10234e320, 0x14000219a40}, {0x140014505c0, 0x1, 0x1}, 0x14002e92160)
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/resolve/loader.go:325 +0x4d8
github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve.(*Loader).resolveAndMergeFetch.func1()
        /Users/dgridnev/go/pkg/mod/github.com/wundergraph/graphql-go-tools/v2@v2.0.0-rc.2.0.20240122172138-bbfa351e99f1/pkg/engine/resolve/loader.go:214 +0xf4
golang.org/x/sync/errgroup.(*Group).Go.func1()
        /Users/dgridnev/go/pkg/mod/golang.org/x/sync@v0.4.0/errgroup/errgroup.go:75 +0x80
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 124
        /Users/dgridnev/go/pkg/mod/golang.org/x/sync@v0.4.0/errgroup/errgroup.go:72 +0xfc

Additional context

[github-actions]

WunderGraph commits fully to Open Source and we want to make sure that we can help you as fast as possible.
The roadmap is driven by our customers and we have to prioritize issues that are important to them.
You can influence the priority by becoming a customer. Please contact us here.

[jensneuse]

Thank you for the detailed report. We'll take a look at the problem. We're currently quite busy so please expect that it might take some time.

[flymedllva]

Can I do that? If not, what is the right way to do it? I have errors that I want to get back online but their status code is 401.

func (m *MyModule) OnOriginResponse(response *http.Response, ctx core.RequestContext) *http.Response {
	response.StatusCode = http.StatusOK
	return nil
}

Sometimes this code produces a panic

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x13ee6a0]

I can assume that response should be checked for nil, but is there an understanding of when it might be nil?

[StarpTech]

HI @flymedllva, I could not reproduce your issue (please try latest) but you need to respect the following case. OnOriginResponse is called concurrently when your operation results in multiple requests. That means you need to protect ctx.ResponseWriter from concurrent access e.g. by a mutex before you can manipulate it.

[flymedllva]

Thanks for the answer!
Sounds logical, but I wish it was written in the documentation, because the current version seems to provide a safe API for module writers. Where should I put the mutex so that it is not for the whole module?

[StarpTech]

You're right. I've updated docs. The current API is the first version since the announcement of the module system. The API is very low-level by purpose. We have waited for this feedback to improve it. Could you elaborate on your use case? Do I understand correctly, that you want to set a header on the final response?

[flymedllva]

In OnOriginRequest, I want request.Header.Set("test", "test") + ctx.ActiveSubgraph
In OnOriginResponse I want to change response.StatusCode = http.StatusOK.
Should I use mutex for request/response?
Where do I put it in ctx? It's not quite clear where to put the mutex

[StarpTech]

I was asking for your use cases. Do you want to set a header to the subgraph request OR do you want to add a header to final client response?

And why do you want to override the response code?

[flymedllva]

At the moment do you want to set a header for the subgraph request but it seems that in the future you will want and do you want to add a header to the final client response

And why do you want to override the response code?

In most cases I need to return a complete response, say when the errors are like this https://www.apollographql.com/docs/apollo-server/data/errors/#built-in-error-codes .
In case of INTERNAL_SERVER_ERROR I want to hide the error and write it to logs.
Besides, my subgraphs can answer 401 except 200, we need it for our metrics and don't want to change to 200 in subgraphs.