Skip to content

Conversation

@dondonz
Copy link
Member

@dondonz dondonz commented Oct 3, 2021

This PR addresses #2560 and improves exception messaging for CoercingParseValueException.

  • Removes nested chain of invalid error:
  • Adds source location back to coercing exception

@dondonz dondonz changed the title WIP: Fix triple nested enum validation error Fix triple nested enum validation error Oct 5, 2021
} else {
Object coercedValue = externalValueToInternalValue(fieldVisibility, variableType, value);
coercedValues.put(variableName, coercedValue);
try {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fundamental change is to move the CoercingParseValueException catch one function earlier in the chain, to externalValueToInternalValueForVariables.

Although it appears many lines are changing, it's mostly only indentation for the try catch block.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale for moving the catch: the source location is available here, whereas it is not passed down to the next function externalValueToInternalValue.

executionResult.data == null
executionResult.errors.size() == 1
executionResult.errors[0].errorType == ErrorType.ValidationError
executionResult.errors[0].message == 'Variable \'input\' has an invalid value: Invalid input for Enum \'PositionType\'. No value found for name \'UNKNOWN_POSITION\''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record - Groovy has 4 types of comments

' single quotes '
" double quotes " 
""" triple double quotes """
''' triple single quotes '''

if you find yourself using `'input' - just swap quotes

@bbakerman bbakerman merged commit 94f0137 into graphql-java:master Oct 7, 2021
@dondonz dondonz deleted the 2560-enum-invalid-error branch October 7, 2021 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants