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

Content edit of error reporting article #4926

Merged
merged 3 commits into from
Mar 1, 2021
Merged

Content edit of error reporting article #4926

merged 3 commits into from
Mar 1, 2021

Conversation

StephenBarlow
Copy link
Contributor

No description provided.

Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

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

Realizing that I actually don't understand AS errors too deeply and might not be best situated to review this. Maybe @trevor-scheer ?


import {ExpansionPanel} from 'gatsby-theme-apollo-docs';

Whenever Apollo Server encounters errors while processing a GraphQL operation, its response to the client includes an `errors` array that contains each error that occurred. Each error in the array has an `extensions` field that provides additional useful information, including an error `code` and an `exception.stacktrace`.
Copy link
Member

Choose a reason for hiding this comment

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

perhaps

and (in development mode) an exception.stacktrace
?


## Masking and logging errors

> To disable stacktraces for production, pass `debug: false` to the Apollo Server constructor or set the `NODE_ENV` environment variable to 'production' or 'test'. Note that this will make the stacktrace unavailable to your application. If you want to log the stacktrace, but not send it in the response to the client, see [Masking and logging errors](#masking-and-logging-errors) below.
Copy link
Member

Choose a reason for hiding this comment

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

It looks a little weird that this entire subsection is a block quote. Is that intentional?

Also I feel like this could be more from a perspective of "stacktraces are on in dev and off in production, by which we mean NODE_ENV is such and such. you can override this by specifying debug: true or debug: false explicitly", if that makes sense?

I also have no clue why they are off by default in test, that seems weird and not particularly explained when it was added in 0e68e2e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah this must be a block I moved while planning to rework, and then forgot about

@glasser
Copy link
Member

glasser commented Feb 26, 2021

Edits LGTM but again I'm not really an expert

const resolvers = {
Query: {
userWithID: (parent, args, context) => {
if (args.id <ExpansionPanel 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Stray paste I suppose

Suggested change
if (args.id <ExpansionPanel 1) {
if (args.id < 1) {

const resolvers = {
Query: {
userWithID: (parent, args, context) => {
if (args.id <ExpansionPanel 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (args.id <ExpansionPanel 1) {
if (args.id < 1) {

Comment on lines 348 to 350
if (err.message.startsWith("Database Error: ")) {
return new Error('Internal server error');
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: use same quote type from one string to the next

},
```

> To make context-specific adjustments to the error received by `formatError` (such as localization or personalization), consider [creating a plugin](../integrations/plugins/) that uses the `didEncounterErrors` lifecycle event to attach additional properties to the error. These properties can be accessed from `formatError`.
Copy link
Member

Choose a reason for hiding this comment

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

I was going to suggest a link specifically to the didEncounterErrors heading on the page, but there's really not much there. Adding an example on modifying errors via that hook would be great as a follow up since visiting that documentation doesn't tell me anything about what I can actually do there. i.e. I wouldn't assume it's ok for me start tacking on properties in that hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I'll add a link anyway, and will make a note to return to this doc soon with an example

@StephenBarlow StephenBarlow merged commit a712208 into main Mar 1, 2021
@StephenBarlow StephenBarlow deleted the sb/error-docs branch March 1, 2021 21:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants