Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic argument validation error is thrown with no indication of the field that is causing it #362

Closed
kfrajtak opened this issue Jun 21, 2019 · 23 comments
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved

Comments

@kfrajtak
Copy link

kfrajtak commented Jun 21, 2019

Describe the bug
Given an InputType field with validation

@InputType()
export class CatalogInput implements Partial<Catalog> {
  @Field()
  id: number;
  @MaxLength(1000)
  @Field({ nullable: true })
  description?: string;
}

and a mutation

@Mutation(returns => CatalogUpdateResult)
updateCatalog(
  @Arg("catalog", () => CatalogInput, { validate: true })
  catalog: CatalogInput
): CatalogUpdateResult {
  return { catalog };
}

when I try to sent mutation to the server with a value exceeding the limit, then I get a very generic error that does not indicate which field caused it, the locations array is empty:

{
  "errors": [
    {
      "message": "Argument Validation Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "updateCatalog"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "errors": [
            {
              "message": "Argument Validation Error",
              "locations": [],
              "path": [
                "updateCatalog"
              ]
            }
          ],
 ...

Expected behavior
I would expected to have the information about what caused the failure, i.e. the location array containing the information.

Enviorment (please complete the following information):

  • OS: Windows
  • Node 10.15.3
  • Package version 0.17.1
  • TypeScript version 3.3.3333

Thanks,
Karel

@MichalLytek MichalLytek added Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue labels Jun 21, 2019
@MichalLytek
Copy link
Owner

@kfrajtak
What is your apollo server version?

When I checked that, it was printing full validation info:
#258 (comment)

@kfrajtak
Copy link
Author

"apollo-server": "^2.4.8"
"apollo-server-core": "^2.5.0"
"apollo-server-express": "^2.4.8"

@MichalLytek
Copy link
Owner

Cannot reproduce with apollo-server v2.4.8 😕

{
  "errors": [
    {
      "message": "Argument Validation Error",
      "locations": [
        {
          "line": 22,
          "column": 3
        }
      ],
      "path": [
        "addRecipe"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "validationErrors": [
            {
              "target": {
                "title": "Correct title",
                "description": "Too short description"
              },
              "value": "Too short description",
              "property": "description",
              "children": [],
              "constraints": {
                "length": "description must be longer than or equal to 30 characters"
              }
            }
          ],
          "stacktrace": [
            "Error: Argument Validation Error",
            "    at Object.<anonymous> (D:\\Projekty\\type-graphql\\src\\resolvers\\validate-arg.ts:29:11)",
            "    at Generator.throw (<anonymous>)",
            "    at rejected (D:\\Projekty\\type-graphql\\node_modules\\tslib\\tslib.js:105:69)",
            "    at processTicksAndRejections (internal/process/task_queues.js:86:5)"
          ]
        }
      }
    }
  ],
  "data": null
}

@kfrajtak
Copy link
Author

I have modified the source code of the ValidationError as follows super("Argument Validation Error " + JSON.stringify(validationErrors)); then I can see that the constraint violation in the output:

...",
"property":"description",
"children":[],
"constraints":{"maxLength":"description must be shorter than or equal to 1000 characters"}}]

Can you share the code you tested it with?

@MichalLytek
Copy link
Owner

Can you share the code you tested it with?

It's master branch of this repo, examples->validation.

@kfrajtak
Copy link
Author

Ok. How to run the example?

@MichalLytek
Copy link
Owner

Just like any other TypeScript Node project.

@kfrajtak
Copy link
Author

If you think tsc index.ts then I get tons of errors, if you think npm run then I don't see package.json.

@MichalLytek
Copy link
Owner

npx ts-node --transpile-only index.ts

@MichalLytek
Copy link
Owner

I've even checked with v2.6.4 and it still works.

@kfrajtak
Copy link
Author

I am sorry, I am not doubting it works, I just want to try to copy the NPM packages to my project ... (I may have made some changes during debugging).

Also it does not work

You need to provide explicit type for Recipe#title !

Installation has some issue too

Error: Cannot find module 'C:\Work\playground\javascript\type-graphql\dist\postinstall'

@kfrajtak
Copy link
Author

Looks like the problem is in Apollo Server when schema stitching is used, see ardatan/graphql-tools#480.

@kfrajtak
Copy link
Author

Hi,
I have successfully run the app and I am getting the error with the information, however the error code is INTERNAL_SERVER_ERROR and not some specific code ...

"errors": [
{
  "message": "Argument Validation Error",
  "locations": [
    {
      "line": 2,
      "column": 3
    }
  ],
  "path": [
    "addRecipe"
  ],
  "extensions": {
    "code": "INTERNAL_SERVER_ERROR",
    "exception": {
      "validationErrors": [
        {
          "target": {
            "title": "X",
            "description": "X"
          },
          "value": "X",
          "property": "description",
          "children": [],
          "constraints": {
            "length": "description must be longer than or equal to 30 characters"
          }
        }
      ],
      "stacktrace": [...]

@MichalLytek
Copy link
Owner

@kfrajtak
Have you tried setting up a custom formatError?

@kfrajtak
Copy link
Author

No, I run your example.
What do you have in mind?

@MichalLytek
Copy link
Owner

Looks like the problem is in Apollo Server when schema stitching is used

I run your example

Please make consistent testimony.

@kfrajtak
Copy link
Author

I haven't modified anything at all, I just installed the packages and run it.
Regarding the stitching - there is number of issues created by users on Apollo server GitHub page and the exceptions cannot cross the stitching boundary unmodified.

@kfrajtak
Copy link
Author

The AuthMiddleware throws either UnauthorizedError or ForbiddenError from your package

throw roles.length === 0 ? new UnauthorizedError() : new ForbiddenError();

Apollo server is using the code inside the error objects to report it back in the error response, see this:

export function toApolloError(
  error: Error & { extensions?: Record<string, any> },
  code: string = 'INTERNAL_SERVER_ERROR',
): Error & { extensions: Record<string, any> } {
  let err = error;
  if (err.extensions) {
    err.extensions.code = code;
  } else {
    err.extensions = { code };
  }
  return err as Error & { extensions: Record<string, any> };
}

So if the code is not set and it looks like none of your error objects have that code defined, then it is not propagated to the toApolloError message.

I would suggest to reuse the errors from the apollo-server-error package, since you are using Apollo server anyway.

@MichalLytek
Copy link
Owner

since you are using Apollo server anyway.

I don't use Apollo Server. TypeGraphQL works with Apollo server, GraphQL Yoga, express-graphql and even Apollo Client. It's weird to change the error for compatibility only with one lib.

You can create your own global middleware that will catch the error and transform it to the format you want.

@kfrajtak
Copy link
Author

I see. The code can help with the distinction.
With that global middleware you meant the Apollo server formatError method?

@MichalLytek
Copy link
Owner

MichalLytek commented Jun 28, 2019

@kfrajtak
Copy link
Author

Great! Thanks for patience and help.

@MichalLytek MichalLytek added Solved ✔️ The issue has been solved and removed Need More Info 🤷‍♂️ Further information is requested labels Jun 30, 2019
@arturasmckwcz

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved
Projects
None yet
Development

No branches or pull requests

3 participants