Allow passing custom headers when persisting queries

[tomgasson]

Specifiy headers in the config (use JS config for dynamic values)

persist: {
  url: "https://persist-service/path",
  headers: {
    Authorization: "bearer TOKEN",
  },
}

And they'll sent along on the request

POST https://persist-service/path
Content-Type: application/x-www-form-urlencoded
Authorization: bearer TOKEN

[maraisr]

why not expose ENV reading support to the config, and something more like;

env AUTH=`echo hello` relay-compiler

"persist": {
    "url": "http://...",
    "headers": {
        "Authorisation": "{ENV.AUTH}"
    }
}

not sure a bespoke command for 1 header is right approach?

[tomgasson]

Open to other options, but that wouldn't work with relay.autoStartCompiler

[captbaritone]

Another possible approach (haven't run this by anyone else on the team) would be to have a third type of persist config which passes the query text to a command via stdin and gets back the id on stdout. That would be a nice way to support arbitrary persisting approaches. Curious if you considered that but rejected, or if there's some challenge to this approach that I'm overlooking.

[tomgasson]

Yeah I thought briefly about that.

There's a few things I thought about:

• The compiler has multiple artifacts to persist, and wants to get a result (id / error) for each of them
• The cost of this (i.e. generating a bearer token), is ideally done once rather than per-query
• Sending multiple queries to a persistence service concurrently (or through some other batching mechanism) rather than sequentially is desirable

We're approaching this problem with a real codebase / service setup of ~500 queries, ~50ms to create the token. This PR leaves us with incremental work at ~1s which is great, and ~20s for a --repersist operation - also fine.

Doing this over stdio would mean a custom encoding mechanism, and I'm not sure if that's worth the additional code cost it would bring.

• Do you have the relay side generate an ID per query, so that the result for each query (which could come in out of order) can be mapped back to the right one (enabling out of order concurrent requests)
• Do you use an index-based system, sending an ordered set of queries and then the other program buffers up results in-order and sends them back once it's all complete. This prevents exiting early on failure, and can force your other command to itself hold all queries in memory

HTTP is actually pretty ideal, since it's already got the concurrency and request->response mapping sorted. We considered a local proxy server to add the Auth, but then you'd have to make sure it's running before running relay - ending up with a wrapper for all calls to relay.

Maybe JSON-RPC (which is already underlying the LSP) over stdio would make sense?

[captbaritone]

Got it, thanks for the detailed explanation. A wrapper shell script which can set environment variables sounds simpler and safer as well. If you are concerned with maintaining compatibility with the VSCode extension, I believe it supports setting a custom compiler path. Would that allow you to use a custom wrapper shell script?

[tomgasson]

I notice that you've apparently got a separately build compiler for Instagram to carry along additional headers e02717c

I'm curious to what sort of info that compiler entrypoint, and FB WWW's compiler entrypoint are carrying along and if those situations would also able to be handled through a wrapper? You've probably got more insight about how these cases could potentially generalise than most.

[voideanvalue]

Open to other options, but that wouldn't work with relay.autoStartCompiler

you could update the vscode extension... ad some config option for executing something before the compiler watch command is executed. I imagine editing https://github.com/facebook/relay/blob/72764d52f382a94d1980c6d4b47cb113be0a0026/vscode-extension/src/config.ts and https://github.com/facebook/relay/blob/72764d52f382a94d1980c6d4b47cb113be0a0026/vscode-extension/src/compiler.ts would allow you to do that.

I notice that you've apparently got a separately build compiler for Instagram to carry along additional headers e02717c

the internal compiler binary uses a custom RemotePersister impl which calls persist with additional headers. the header key/value pair are generated within the compiler binary and aren't based on anything from the config.

[alunyov]

what do you think about adding new configuration section similar to params, but for headers:

And the dynamic values to these headers can be passed from the relay.config.js - dynamic JS config where you can call to that authorization command to generate tokens/headers.

[tomgasson]

That's a much simpler solution - thanks for pointing it out. Updated to that

[facebook-github-bot]

@alunyov has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.