-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Describe the bug
I just hit a bug in production when deploying an upgrade from 18.0 to 18.1 that I think relates to some of the work in resolving #2811
I noticed the following in my production logs
...
Caused by: graphql.schema.CoercingParseValueException: Variable 'memberDues' has an invalid value: Failed to parse input value 2022-05-16T19:52:37Z as ZonedDateTime
...
Caused by: java.lang.IllegalArgumentException: Unexpected input type: class java.time.ZonedDateTimeat com.meetup.graphql.wiring.CustomScalars.parseDateTimeValue(CustomScalars.java:38)at com.meetup.graphql.wiring.CustomScalars$1.parseValue(CustomScalars.java:71)at com.meetup.graphql.wiring.CustomScalars$1.parseValue(CustomScalars.java:63)at graphql.execution.ValuesResolver.externalValueToInternalValueForScalar(ValuesResolver.java:578)at graphql.execution.ValuesResolver.externalValueToInternalValue(ValuesResolver.java:510)at graphql.execution.ValuesResolver.externalValueToInternalValue(ValuesResolver.java:498)at graphql.execution.ValuesResolver.externalValueToInternalValueForObject(ValuesResolver.java:565)at graphql.execution.ValuesResolver.externalValueToInternalValue(ValuesResolver.java:517)at graphql.execution.ValuesResolver.externalValueToInternalValueForObject(ValuesResolver.java:565)at graphql.execution.ValuesResolver.externalValueToInternalValue(ValuesResolver.java:517)at graphql.execution.ValuesResolver.externalValueToInternalValue(ValuesResolver.java:498)at graphql.execution.ValuesResolver.externalValueToInternalValueForVariables(ValuesResolver.java:410)
note that the string 2022-05-16T19:52:37Z is actually parsable as ZonedDateTime however I believe the issue is previously parseValue was receiving either a String or a StringValue and after this upgrade it started receiving the value that I was attempting to coerce it into, in this case a ZonedDateTime
is this new expected behavior or was this previous expected behavior that was underspecified until now?
Here's an abbreviated version of a custom scaler for parsing out ZonedDateTime values which is roughly based on the docs
static ZonedDateTime parseDateTimeValue(Object input) {
try {
if (input instanceof StringValue) {
return ZonedDateTime.parse(((StringValue) input).getValue());
} else if (input instanceof String) {
return ZonedDateTime.parse((String) input);
} else throw new IllegalArgumentException("Unexpected input type: " + input.getClass()); <-- change in behavior causes this line to get evaluated
} catch (Exception e) {
throw new CoercingParseValueException(
"Failed to parse input value " + input + " as ZonedDateTime", e);
}
}
public static final GraphQLScalarType DateTime =
GraphQLScalarType.newScalar()
.name("DateTime")
.description("A custom scalar that handles timestamps")
.coercing(
new Coercing<ZonedDateTime, String>() {
@Override
public String serialize(Object dataFetcherResult) {
return // doesnt matter
}
@Override
public ZonedDateTime parseValue(Object input) {
return parseDateTimeValue(input);
}
@Override
public ZonedDateTime parseLiteral(Object input) {
return // doesnt matter
}
})
.build();To Reproduce
Please provide a code example or even better a test to reproduce the bug.
see above