Subgraph validation error of custom scalars shows the normalised variable name.

[dwjohnston]

Component(s)

router

Component version

0.321.1

wgc version

0.129.3

controlplane version

cloud

router version

0.321.1

What happened?

If I have a subgraph that uses a custom sclar like

# format 'anystring-anystring`
scalar MyCustomScalar 

type Query {
    myquery(value: MyCustomScalar!): MyCustomScalar
}

And I query this with

query MyQuery($customValue: MyCustomScalar!) {
    myquery(value: $customValue)
}

with variables:

{
    "customValue": "foo--bar"
}

Then I see an error like

Variable "$a" got invalid value "foo--bar"; Expected type "MyCustomScalar".

I believe this is an issue with the query normalisation/denormalisation.

This makes it that little bit harder to debug what's wrong with the query.

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Package Manager: pnpm, npm, yarn, etc
Compiler(if manually compiled): (e.g., "go 14.2")

Router configuration

Router execution config

Log output

Additional context

Related configuration: #590

AI Notes

(🙋‍♂️ I personally have not verified any of this)

The variable rename itself happens in graphql-go-tools here:

v2/pkg/astnormalization/variables_mapping.go, lines 71–84.

The key lines are:

mappingName := v.generateUnusedVariableMappingName()

v.mapping[string(mappingName)] = variableItem.variableName
newVariableName := v.operation.Input.AppendInputBytes(mappingName)

// set new variable name for all variable values
for _, variableValueRef := range variableItem.valueRefs {
    v.operation.VariableValues[variableValueRef].Name = newVariableName
}

// set new variable name for variable definition
v.operation.VariableValues[
    v.operation.VariableDefinitions[variableItem.variableDefinitionRef].VariableValue.Ref
].Name = newVariableName

In particular:

v.mapping[string(mappingName)] = variableItem.variableName

means the mapping is already exactly what would be needed to reverse the error:

a -> brandedStoreId

Then Cosmo calls that mapper in:

router/core/operation_processor.go, lines 1016–1021.

if !disabled {
    variablesMap := o.kit.variablesRemapper.NormalizeOperation(
        o.kit.doc,
        o.operationProcessor.executor.ClientSchema,
        report,
    )

    ...

    o.parsedOperation.RemapVariables = variablesMap
}

So Cosmo does retain the mapping.

It then explicitly passes that mapping into the execution context:

router/core/graphql_handler.go, lines 1785–1791. ([GitHub]1)

resolveCtx := resolve.NewContext(executionContext)

resolveCtx.Variables = reqCtx.operation.variables
resolveCtx.RemapVariables = reqCtx.operation.remapVariables
resolveCtx.VariablesHash = reqCtx.operation.variablesHash

And graphql-go-tools documents exactly what it means:

v2/pkg/engine/resolve/context.go, lines 20–27:

// RemapVariables contains a map from new names to old names. When variables are renamed,
// the resolver will use the new name to look up the old name to render the variable in the query.
RemapVariables map[string]string

Where I think the actual gap is

The most interesting code is:

v2/pkg/engine/resolve/loader.go → func (l *Loader) mergeErrors(...), starting around line 3759. ([GitHub]2)

It receives the subgraph errors:

func (l *Loader) mergeErrors(
    res *result,
    fetchItem *FetchItem,
    value *astjson.Value,
) error {
    values := value.GetArray()

Then it performs several transformations:

l.optionallyOmitErrorLocations(values)

if l.rewriteSubgraphErrorPaths {
    rewriteErrorPaths(...)
}

l.optionallyEnsureExtensionErrorCode(values)

...

But there is nothing equivalent for variable names inside message. ([GitHub]2)

Then in pass-through mode it ultimately does:

// If the error propagation mode is pass-through, we append the errors to the root array
l.errors.AppendArrayItems(l.jsonArena, value)

at roughly 3831–3833. ([GitHub]2)

So the flow appears to be:

$brandedStoreId
       │
       ▼
variables_mapping.go
       │
       ├── changes query to $a
       │
       └── retains map: a -> brandedStoreId
       │
       ▼
subgraph receives $a
       │
       ▼
subgraph error:
"Variable \"$a\" got invalid value..."
       │
       ▼
Loader.mergeErrors()
       │
       ├── rewrite paths        ✓
       ├── rewrite extensions   ✓
       ├── rewrite locations    ✓
       └── rewrite message vars ✗
       │
       ▼
AppendArrayItems(...)
       │
       ▼
client sees "$a"

So if I were pointing the WunderGraph maintainers at one place to investigate, it would be:

graphql-go-tools/v2/pkg/engine/resolve/loader.go, Loader.mergeErrors() around lines 3759–3833.

The remapping machinery is already available on l.ctx.RemapVariables; what's missing appears to be applying it to downstream error messages before those errors are appended to the client response.

That would also be useful to include in your GitHub issue as something like: “I traced this to Loader.mergeErrors; Context.RemapVariables already contains the new→old mapping, but it does not appear to be applied when propagating downstream error messages.”

[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.