#Code first graphql and structural typing woes

22 messages · Page 1 of 1 (latest)

neon rune
#

I've got a repro here:

Basically, the modifyFoo payload is supposed to only accept FooFailure as a type, however, it accepts BarFailure because the signature is the same. Even the error part of it, the signature is the same.

This makes for runtime errors that are pretty difficult to see in review.

Obviously we can add a random property (like fooFailure = true) to FooFailure, and then it works as desired because the signature is different, but requiring a unique structure will also be hard to catch in review.

Anyone know of a way to work around this?

import { Field, Mutation, ObjectType, Resolver, createUnionType } from '@nestjs/graphql';

@ObjectType()
class FirstError { title = "foo"; message = "bar"; }

@ObjectType()
class OtherError { title = "bar"; message = "foo"; }

const FooError = createUnionType({
  name: 'FooError',
  types: () => [FirstError],
});

const BarError = createUnionType({
  name: 'BarError',
  types: () => [OtherError],
});

@ObjectType()
class FooFailure {
  @Field(() => FooError)
  error: typeof FooError;

  constructor(error: typeof FooError) {
    this.error = error;
  }
}

@ObjectType()
class BarFailure {
  @Field(() => BarError)
  error: typeof BarError;

  constructor(error: typeof BarError) {
    this.error = error;
  }
}

export const FooPayload = createUnionType({
  name: 'FooPayload',
  types: () => [FooFailure],
})

export const BarPayload = createUnionType({
  name: 'BarPayload',
  types: () => [BarFailure],
});


@Resolver()
export class FooBarResolver {
  @Mutation(() => FooPayload)
  modifyFoo(): typeof FooPayload {
    return new BarFailure(new OtherError())
  }
}
neon rune
#

Perhaps there's a way to make an @ObjectType wrapper that adds a unique, non-graphql exposed field just so the structure is different. I'm too much of a typescript noob to know how to do that, though

hushed chasm
#

I'm not certain what you are trying to accomplish, but if the error types all have the same structure, why not just use one error type?

neon rune
#

they have the same structure in typescript, but they are not the same in graphql. the client can easily determine the type of the error and act accordingly

#

so, even if every error just has title and message, they can still tell the difference between FirstError and OtherError .

Furthermore, even though the app compiles, there is a runtime crash when you return an type that is not part of the union in graphql. What would be great here is a way to determine that someone has copy/pasted a problem that you may not see for a long time

hushed chasm
#

I'm not the most experienced in GraphQL, but I've never heard of figuring out the error via its type.

neon rune
hushed chasm
#

Yeah, so I'm still not sure of the problem. If you are doing a union to work in errors as part of the return data, the union should entail the resulting object for a successful mutation (or query) and an error object for the failure. The object names must be unique.

So, for example:

@ObjectType()
class Token {
 @Field() 
 token: string
}

@ObjectType()
class AuthError {
 @Field()
 message: string
}

const AuthPayload = createUnionType({
 name: 'AuthPayload',
 types: () => [Token, AuthError],
});

@InputType()
export class LoginInput {
 @Field()
 username: string;

 @Field()
 password: string;
}

@Resolver()
export class AuthResolver {
 @Mutation(() => AuthPayload)
 auth(@Args('loginData') loginData: LoginInput): AuthPayload {
   // do something to login user
   if (error) return AuthError
   return Token
 }
}

On the client you would use fragments to capture the two possible result objects.

mutation {
 auth(username: "demo", password: "demo") {
   ... on Token {
     token
   }
   ... on AuthError {
     message
   }
 }
}

So, with the tools at hand, I'm uncertain how you would catch an error by type.

neon rune
#

The problem is that if the errors have the same shape as each other, i can return an error that is not part of the graphql union in typescript, because typescript only cares about the shape. It will compile, everything will be fine until the error state is reached, and then it will crash, because graphql cares about the name, not the shape. The crash itself comes from Apollo, this is not the client crashing, this is apollo saying "this union is made up of FooError | BarError, but you returned SomeOtherError"

We have fixed it by adding the random property. however, this is error prone because the compiler doesn’t complain if you forget to add it. Now you’ve got a situation where you can accidentally return the wrong error and the compiler will not see it. You will only know about the problem once the error state is reached

dire harness
#

I went with manual discriminator field (or how to call that). Sure, in review you could miss that it's not defined, but it works well (type-safety wise) thorough the app.

Screenshot attached to illustrate that it's not too complicated to spot an error, if you have all the errors defined in single file. I've also tidied up in the 2nd screen, for less noise.

I believe this could even be refactored to get rid of the duplicate name, but I haven't found how yet. Since we don't add many new errors, and it works great otherwise, I just didn't burn more time on that 😁

#

Oh, maybe you're just missing the as const on your existing title = 'foo' as const property. That should be enough for TS to discriminate on them.

neon rune
#

it works without the const. just one of those things where type safety can’t really be enforced by the compiler i guess

#

discriminator is probably good enough. this problem hasn’t come up a ton, it’s just another one of those annoying TS oddities

#

(don’t get me wrong, i would be very sad without typescript)

#

Is the terminology is that graphql has nominal types, typescript doesn’t? there is a bit of a conflict between the two type systems that can be a footgun

dire harness
#

there is a runtime crash when you return an type that is not part of the union in graphql
Is this the problem? Because I get an error at compile time when I do that.

neon rune
#

even if it’s not discriminated? Did you try the example at the top of this post?

#

what was your discriminator added to solve if not this?

dire harness
neon rune
#

yeah that’s what i’m referring to. what happens if you remove the kind property, and all the errors have the same shape? do you get a runtime or a compile time error?

#

(no worries, not in any hurry or anything)

dire harness
#

No compile time error without the kind property when all the errors have the same shape (tested). I believe it will be runtime graphql error saying it's not part of the union (not tested).