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

make errors as extensions of GraphQLError #62

Closed
zsolt-dev opened this issue Mar 28, 2018 · 5 comments
Closed

make errors as extensions of GraphQLError #62

zsolt-dev opened this issue Mar 28, 2018 · 5 comments

Comments

@zsolt-dev
Copy link

When I upload a file bigger than maxFileSize the entire app crashes with this error:

MaxFileSizeUploadError: File truncated as it exceeds the size limit.

would it be better to throw GraphQLError instead so user will get a feedback and not crash the server?

@jaydenseric
Copy link
Owner

Duplicates #45.

@zsolt-dev
Copy link
Author

I have read the #45 before and updated to the latest version. But it is not throwing the error type: GraphQLError. If it would trow GraphQLError the resolver would return it as an error to the client and not crash the api.

@jaydenseric
Copy link
Owner

We need a way to handle the error as a stream event, so that when an upload stream to a cloud service suddenly cuts off due to the file size limit it can be handled properly (cleanup, etc.).

It is really up to the developer how the error should be communicated to the client; I don't throw validation errors in my resolvers (resulting in a GraphQL error). Instead I populate an errors field in the response payload. Something like this.

@zsolt-dev
Copy link
Author

Thank you for your response.

I still do not know how to know if en error is from your library or not.

copy&paste from your link:

  if (
    // Image stream was truncated due to maxFileSize
  ) payload.errors.image.push('Filesize too big.')

how do I know the Image stream was truncated due to maxFileSize or some other error was thrown from your library? Where should I do the try{} catch{} ?

It would be nice if the I could catch error here like this and re-throw it:

    try {
      const { stream, filename, mimetype, encoding } = await data.image;
    } catch (error) {
      throw new GraphQLError(error.message);
    }

Sorry again for lame questions I am new to node/graphql...

Thank you very much for taking your time of your busy day doing great packages.

@jaydenseric
Copy link
Owner

In theory, you should be able to do this:

// Individual file upload stream
stream.on('error', error => {
  if (stream.truncated) console.log('Cut off for some reason.')
  // Or…
  if (error.name === 'MaxFileSizeUploadError') console.log('Cut off specifically due to exceeded max file size')
})

But, there is that bug causing the server to crash instead.

One possible fix I could do is to change the API, so that the MaxFileSizeUploadError does not cause an error event:

stream.on('limit', error => { })

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

No branches or pull requests

2 participants