Skip to content

regression in GraphQLScalarType Coercing parseValue in 18.1 #2819

@softprops

Description

@softprops

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions