Add BadUserInputError and TransientError as extensions of ApolloError #1143

clarencenpy · 2018-06-05T08:41:24Z

Proposed additions of two new error types. They are just simple wrappers around the ApolloError interface, and could be used to highlight more use cases of using ApolloErrors.

TODO:

Update CHANGELOG.md with your change (include reference to issue & this PR)
Make sure all of the significant new logic is covered by tests
Rebase your changes on master so that they can be merged easily
Make sure all tests and linter rules pass

evans

BadUserInputError is a great addition! Adding it and the other errors to the api reference would be awesome too. Pointing to the error reference section in the error handling section would make them more visible as well!

I think for now TransientError would be better as a suggested pattern, rather than a baked in error. Currently the name seems a bit too general and its functionality isn't super well specified. As we build the capabilities into apollo server, such as rate limiting, it could be confusing.

evans · 2018-06-05T22:46:19Z

packages/apollo-server-core/src/errors.ts

+  }
+}
+
+export class TransientError extends ApolloError {


Let's leave the TransientError out for now, since we don't have a compelling use case on the client-side. It seems like the best place to handle theses sorts of errors is either in the layer above the GraphQL execution, such as middleware, or in the Data sources, which could retry on their own. I totally agree that we want this sort of retriability. I'm just not convinced that an error in the resolvers is the right place.

I agree with you. Let's keep it as a suggested pattern so that our use cases for ApolloError stays solid!

Working updating the error handling section of the docs as well. What do you feel about moving some of the code examples from my blog over as well once they have been reviewed?

Totally! We can move them over now too if you prefer

evans · 2018-06-05T22:52:36Z

packages/apollo-server-core/src/errors.test.ts

+        errorClass: BadUserInputError,
+        name: 'BadUserInputError',
+      });
+      console.log(`error: ${JSON.stringify(error, null, 2)}`);


We should remove logs from the code. Ideally it would be caught in the linting step. I guess prettier doesn't do that for us anymore 😢

Yes thanks for catching that🙏🙏

evans · 2018-06-05T23:14:26Z

We should also make sure that the base branch is version-2, rather than the old refactor-2.0

clarencenpy · 2018-06-06T21:27:29Z

@evans the tests are now failing because I updated it so that it checks for properties that was passed in ApolloError() in extensions.exception instead of the root level of the error.

evans

@clarencenpy Great stuff! After we get the tests passing, plus new screenshot and a couple cosmetic changes to the code samples, we'll be all set.

evans · 2018-06-07T22:20:40Z

docs/source/features/errors.md

@@ -101,12 +96,20 @@ The response will return:

 ![Screenshot demonstrating augmented error](../images/features/error-user-input.png)


We should update this screenshot to match the new code

evans · 2018-06-07T22:23:11Z

docs/source/features/errors.md

@@ -43,11 +42,7 @@ The response will return:
 In addition to stacktraces, Apollo Server's exported errors specify a human-readable string in the `code` field of `extensions` that enables the client to perform corrective actions. In addition to improving the client experience, the `code` field allows the server to categorize errors. For example, an `AuthenticationError` sets the code to `UNAUTHENTICATED`, which enables the client to reauthenticate and would generally be ignored as a server anomaly.

 ```js line=4,15-17


These highlighted lines should line up with the AuthenticationError and it's usage, so I think we want to keep them on separate lines even though it might not match Prettier's styling

evans · 2018-06-07T22:25:52Z

packages/apollo-server-core/src/errors.test.ts

+        errorClass: BadUserInputError,
+        name: 'BadUserInputError',
+      });
+      expect(error.extensions.exception.field1).to.equal('property1');


We can make these fields on the root error. The important part is for when these errors are thrown in resolvers to contain the correct layout. Here for example

clarencenpy · 2018-06-08T02:49:31Z

@evans should be good to go!

evans · 2018-06-08T17:39:57Z

@clarencenpy Awesome work! We'll release this in the next beta 🎉

abernix · 2018-06-12T11:51:42Z

Purely talking points:

Is there a particular reason for the Bad prefix on BadUserInputError? It would seem that UserInputError might still accurately represent a user-input error.
Since these are all being exported from apollo-server, and are presumably all extended versions of ApolloError, would it be at all beneficial to prefix them with ApolloError (e.g. ApolloErrorUserInput, etc.). (Thinking about intellisense/auto-completion/discoverability)

clarencenpy · 2018-06-15T18:07:25Z

@abernix My apologies, should have seen your comments earlier!

(copied from slack) I see your point there! I guess one reason why that naming makes sense is that it maps directly to the error code in the response, which is BAD_USER_INPUT. The convention is similar for the other error types.
I'm all for this if we dont think the names get too verbose? Intellisense does offer completions when we type error in the import { ... } from 'apollo-server' statement though, that might be good enough for discoverability?

Add BadUserInputError and TransientError as extensions of ApolloError

bee0fe7

clarencenpy requested a review from evans June 5, 2018 08:41

ghost added the ⛲️ feature New addition or enhancement to existing solutions label Jun 5, 2018

evans reviewed Jun 5, 2018

View reviewed changes

evans changed the base branch from refactor-2.0 to version-2 June 5, 2018 23:14

ghost added the ⛲️ feature New addition or enhancement to existing solutions label Jun 5, 2018

evans added this to the Release 2.0 milestone Jun 6, 2018

clarencenpy added 2 commits June 6, 2018 14:19

remove TransientError

02e6ab3

update test so that properties end up under error.extensions.exception

36563f6

update documentation to talk about BadUserInputError

090af9f

evans reviewed Jun 7, 2018

View reviewed changes

clarencenpy and others added 8 commits June 7, 2018 17:22

check for properties after formatApolloErrors has been called

0b3f1e5

fixed highlighted lines for docs

e7784e7

Merge branch 'version-2' into clarencenpy/proposed-error-types

beed3c3

export BadUserInputError from apollo-server

b86778a

rebase onto version-2

ae320e4

fixed highlighted lines for docs

b0ca232

export BadUserInputError from apollo-server

9235f96

update changelog

fa96b01

ghost added the ⛲️ feature New addition or enhancement to existing solutions label Jun 8, 2018

evans merged commit 5f30792 into version-2 Jun 8, 2018

clarencenpy deleted the clarencenpy/proposed-error-types branch June 19, 2018 18:19

github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BadUserInputError and TransientError as extensions of ApolloError #1143

Add BadUserInputError and TransientError as extensions of ApolloError #1143

clarencenpy commented Jun 5, 2018 •

edited

Loading

evans left a comment

evans Jun 5, 2018

clarencenpy Jun 5, 2018

clarencenpy Jun 5, 2018

evans Jun 6, 2018

evans Jun 5, 2018

clarencenpy Jun 5, 2018

evans commented Jun 5, 2018

clarencenpy commented Jun 6, 2018

evans left a comment

evans Jun 7, 2018

evans Jun 7, 2018

evans Jun 7, 2018

clarencenpy commented Jun 8, 2018

evans commented Jun 8, 2018

abernix commented Jun 12, 2018

clarencenpy commented Jun 15, 2018

		@@ -101,12 +96,20 @@ The response will return:

		![Screenshot demonstrating augmented error](../images/features/error-user-input.png)

		@@ -43,11 +42,7 @@ The response will return:
		In addition to stacktraces, Apollo Server's exported errors specify a human-readable string in the `code` field of `extensions` that enables the client to perform corrective actions. In addition to improving the client experience, the `code` field allows the server to categorize errors. For example, an `AuthenticationError` sets the code to `UNAUTHENTICATED`, which enables the client to reauthenticate and would generally be ignored as a server anomaly.

		```js line=4,15-17

Add BadUserInputError and TransientError as extensions of ApolloError #1143

Add BadUserInputError and TransientError as extensions of ApolloError #1143

Conversation

clarencenpy commented Jun 5, 2018 • edited Loading

evans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evans commented Jun 5, 2018

clarencenpy commented Jun 6, 2018

evans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarencenpy commented Jun 8, 2018

evans commented Jun 8, 2018

abernix commented Jun 12, 2018

clarencenpy commented Jun 15, 2018

clarencenpy commented Jun 5, 2018 •

edited

Loading