Skip to content

Conversation

@ryanruanwork
Copy link
Contributor

@ryanruanwork ryanruanwork commented Mar 17, 2020

Right now, dataloader-codegen will throw error when the number of items in request and response are not the same. see https://github.com/Yelp/dataloader-codegen/blob/master/src/implementation.ts#L249-L254

But given the nature of different implementation of resources(endpoints), developers using dataloader-codegen might want to catch this error in some cases.

For example, there is a resource named getUserMessage, it returns an empty array whenuserNotExist, this is an expected behaviour of the resource, but dataloader-codegen will throw a BatchItemNotFoundError. Developers may want to catch this error and return 0.

So, in this PR, I am going to export BatchItemNotFoundError error, so that developers will be able to catch this kind of error and return the default value they want.

@ryanruanwork ryanruanwork requested a review from magicmark March 17, 2020 22:13
@magicmark
Copy link
Collaborator

magicmark commented Mar 18, 2020

Are you sure we need to actually import apollo-server-express? As long as we can produce an error object that contains the code in the right place, we should be ok right?:

> const ApolloError = require('apollo-server-express').ApolloError;
undefined
> class FooError extends ApolloError{ constructor() { super('foo', 'FOO_BAR_BAZ_ERROR'); } }
undefined
> const e = new FooError('yikes')
undefined
> Object.keys(e)
[ 'extensions' ]
> Object.keys(e.extensions)
[ 'code' ]
> e.extensions.code
'FOO_BAR_BAZ_ERROR'

It looks like Apollo Server gets the error code by looking at .extensions.code - can we just add this to our error object?

also - could you provide a little more context in the PR description? What precisely are we trying to achieve here?

@ryanruanwork ryanruanwork force-pushed the extends_apollo_error branch from a586342 to 5dda414 Compare March 18, 2020 01:52
@ryanruanwork ryanruanwork changed the title extends apollo error add extension.code field to BatchItemNotFoundError Mar 18, 2020
Copy link
Collaborator

@magicmark magicmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

@ryanruanwork ryanruanwork merged commit 0e5ea7d into master Mar 18, 2020
@ryanruanwork ryanruanwork deleted the extends_apollo_error branch March 18, 2020 18:07
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

Successfully merging this pull request may close these issues.

3 participants