Skip to content

Commit

Permalink
includeStacktrace -> debug
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie committed Aug 6, 2021
1 parent 6631b94 commit 208ae7a
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 35 deletions.
4 changes: 2 additions & 2 deletions packages/keystone/src/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function getCommittedArtifacts(
graphQLSchema: GraphQLSchema,
config: KeystoneConfig
): Promise<CommittedArtifacts> {
const errors = graphQlErrors(config.graphql?.includeStacktrace);
const errors = graphQlErrors(config.graphql?.debug);
const lists = initialiseLists(config.lists, errors, getDBProvider(config.db));
const prismaSchema = printPrismaSchema(
lists,
Expand Down Expand Up @@ -179,7 +179,7 @@ export async function generateNodeModulesArtifacts(
) {
const lists = initialiseLists(
config.lists,
graphQlErrors(config.graphql?.includeStacktrace),
graphQlErrors(config.graphql?.debug),
getDBProvider(config.db)
);

Expand Down
4 changes: 2 additions & 2 deletions packages/keystone/src/lib/core/graphql-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export type KeystoneErrors = {
};

// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const graphQlErrors = (includeStacktrace: boolean | undefined): KeystoneErrors => ({
export const graphQlErrors = (debug: boolean | undefined): KeystoneErrors => ({
accessDeniedError: () => new ApolloError('You do not have access to this resource'),
validationFailureError: (messages: string[]) => {
const s = messages.map(m => ` - ${m}`).join('\n');
Expand All @@ -20,7 +20,7 @@ export const graphQlErrors = (includeStacktrace: boolean | undefined): KeystoneE
`An error occured while running "${extension}".\n${s}`,
'INTERNAL_SERVER_ERROR',
// Make the original stack traces available in non-production modes.
(includeStacktrace === undefined ? process.env.NODE_ENV !== 'production' : includeStacktrace)
(debug === undefined ? process.env.NODE_ENV !== 'production' : debug)
? { debug: things.map(t => ({ stacktrace: t.error.stack, message: t.error.message })) }
: undefined
);
Expand Down
2 changes: 1 addition & 1 deletion packages/keystone/src/lib/createSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function getInternalGraphQLSchema(
export function createSystem(config: KeystoneConfig) {
const provider = getDBProvider(config.db);

const errors = graphQlErrors(config.graphql?.includeStacktrace);
const errors = graphQlErrors(config.graphql?.debug);

const lists = initialiseLists(config.lists, errors, provider);

Expand Down
2 changes: 1 addition & 1 deletion packages/keystone/src/lib/server/createApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const _createApolloServerConfig = ({
return {
uploads: false,
schema: graphQLSchema,
debug: graphqlConfig?.includeStacktrace, // If undefined, use Apollo default of NODE_ENV !== 'production'
debug: graphqlConfig?.debug, // If undefined, use Apollo default of NODE_ENV !== 'production'
// FIXME: support for apollo studio tracing
// ...(process.env.ENGINE_API_KEY || process.env.APOLLO_KEY
// ? { tracing: true }
Expand Down
8 changes: 4 additions & 4 deletions packages/types/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,15 @@ export type GraphQLConfig = {
* In general both categories of stacktrace are useful for debugging while developing,
* but should not be exposed in production, and this is the default behaviour of Keystone.
*
* You can use the `includeStacktrace` option to change this behaviour. A use case for this
* You can use the `debug` option to change this behaviour. A use case for this
* would be if you need to send the stacktraces to a log, but do not want to return them
* from the API. In this case you could set `includeStacktrace: true` and use
* from the API. In this case you could set `debug: true` and use
* `apolloConfig.formatError` to log the stacktraces and then strip them out before
* returning the error.
*
* ```
* graphql: {
* includeStacktrace: true,
* debug: true,
* apolloConfig: {
* formatError: err => {
* console.error(err);
Expand All @@ -176,7 +176,7 @@ export type GraphQLConfig = {
* *
* Default: process.env.NODE_ENV !== 'production'
*/
includeStacktrace?: boolean;
debug?: boolean;
};

// config.extendGraphqlSchema
Expand Down
40 changes: 20 additions & 20 deletions tests/api-tests/hooks/hook-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createSchema, list } from '@keystone-next/keystone/schema';
import { setupTestRunner } from '@keystone-next/testing';
import { apiTestConfig, expectAccessDenied, expectExtensionError } from '../utils';

const runner = (includeStacktrace: boolean | undefined) =>
const runner = (debug: boolean | undefined) =>
setupTestRunner({
config: apiTestConfig({
lists: createSchema({
Expand Down Expand Up @@ -85,13 +85,13 @@ const runner = (includeStacktrace: boolean | undefined) =>
},
}),
}),
graphql: { includeStacktrace },
graphql: { debug },
}),
});

[true, false, undefined].map(includeStacktrace => {
[true, false, undefined].map(debug => {
(['dev', 'production'] as const).map(mode =>
describe(`NODE_ENV=${mode}, includeStacktrace=${includeStacktrace}`, () => {
describe(`NODE_ENV=${mode}, debug=${debug}`, () => {
beforeAll(() => {
// @ts-ignore
process.env.NODE_ENV = mode;
Expand All @@ -105,7 +105,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
describe(`List Hooks: ${phase}Change/${phase}Delete()`, () => {
test(
'createOne',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Valid name should pass
await context.lists.User.createOne({ data: { name: 'good' } });

Expand All @@ -118,7 +118,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
// Returns null and throws an error
expect(data).toEqual({ createUser: null });
const message = `Simulated error: ${phase}Change`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Change`, [
expectExtensionError(mode, false, debug, errors, `${phase}Change`, [
{
path: ['createUser'],
messages: [`User: Simulated error: ${phase}Change`],
Expand All @@ -143,7 +143,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
'updateOne',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Valid name should pass
const user = await context.lists.User.createOne({ data: { name: 'good' } });
await context.lists.User.updateOne({
Expand All @@ -160,7 +160,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
// Returns null and throws an error
expect(data).toEqual({ updateUser: null });
const message = `Simulated error: ${phase}Change`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Change`, [
expectExtensionError(mode, false, debug, errors, `${phase}Change`, [
{
path: ['updateUser'],
messages: [`User: ${message}`],
Expand All @@ -185,7 +185,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
'deleteOne',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Valid names should pass
const user1 = await context.lists.User.createOne({ data: { name: 'good' } });
const user2 = await context.lists.User.createOne({
Expand All @@ -202,7 +202,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
// Returns null and throws an error
expect(data).toEqual({ deleteUser: null });
const message = `Simulated error: ${phase}Delete`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Delete`, [
expectExtensionError(mode, false, debug, errors, `${phase}Delete`, [
{
path: ['deleteUser'],
messages: [`User: ${message}`],
Expand All @@ -227,7 +227,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
'createMany',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Mix of good and bad names
const { data, errors } = await context.graphql.raw({
query: `mutation ($data: [UserCreateInput!]!) { createUsers(data: $data) { id name } }`,
Expand All @@ -254,7 +254,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
});
// The invalid creates should have errors which point to the nulls in their path
const message = `Simulated error: ${phase}Change`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Change`, [
expectExtensionError(mode, false, debug, errors, `${phase}Change`, [
{
path: ['createUsers', 1],
messages: [`User: ${message}`],
Expand Down Expand Up @@ -296,7 +296,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
'updateMany',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Start with some users
const users = await context.lists.User.createMany({
data: [
Expand Down Expand Up @@ -333,7 +333,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
});
// The invalid updates should have errors which point to the nulls in their path
const message = `Simulated error: ${phase}Change`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Change`, [
expectExtensionError(mode, false, debug, errors, `${phase}Change`, [
{
path: ['updateUsers', 1],
messages: [`User: ${message}`],
Expand Down Expand Up @@ -375,7 +375,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
'deleteMany',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
// Start with some users
const users = await context.lists.User.createMany({
data: [
Expand Down Expand Up @@ -407,7 +407,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
});
// The invalid deletes should have errors which point to the nulls in their path
const message = `Simulated error: ${phase}Delete`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Delete`, [
expectExtensionError(mode, false, debug, errors, `${phase}Delete`, [
{
path: ['deleteUsers', 1],
messages: [`User: ${message}`],
Expand Down Expand Up @@ -453,7 +453,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
describe(`Field Hooks: ${phase}Change/${phase}Delete()`, () => {
test(
'update',
runner(includeStacktrace)(async ({ context }) => {
runner(debug)(async ({ context }) => {
const post = await context.lists.Post.createOne({
data: { title: 'original title', content: 'original content' },
});
Expand All @@ -467,7 +467,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
});
const message1 = `Simulated error: title: ${phase}Change`;
const message2 = `Simulated error: content: ${phase}Change`;
expectExtensionError(mode, false, includeStacktrace, errors, `${phase}Change`, [
expectExtensionError(mode, false, debug, errors, `${phase}Change`, [
{
path: ['updatePost'],
messages: [`Post.title: ${message1}`, `Post.content: ${message2}`],
Expand Down Expand Up @@ -504,7 +504,7 @@ const runner = (includeStacktrace: boolean | undefined) =>

test(
`delete`,
runner(includeStacktrace)(async ({ context, graphQLRequest }) => {
runner(debug)(async ({ context, graphQLRequest }) => {
const post = await context.lists.Post.createOne({
data: { title: `trigger ${phase} delete`, content: `trigger ${phase} delete` },
});
Expand All @@ -515,7 +515,7 @@ const runner = (includeStacktrace: boolean | undefined) =>
const { data, errors } = body;
const message1 = `Simulated error: title: ${phase}Delete`;
const message2 = `Simulated error: content: ${phase}Delete`;
expectExtensionError(mode, true, includeStacktrace, errors, `${phase}Delete`, [
expectExtensionError(mode, true, debug, errors, `${phase}Delete`, [
{
path: ['deletePost'],
messages: [`Post.title: ${message1}`, `Post.content: ${message2}`],
Expand Down
9 changes: 4 additions & 5 deletions tests/api-tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const expectValidationError = (
export const expectExtensionError = (
mode: 'dev' | 'production',
httpQuery: boolean,
includeStacktrace: boolean | undefined,
_debug: boolean | undefined,
errors: readonly any[] | undefined,
extensionName: string,
args: { path: (string | number)[]; messages: string[]; debug: any[] }[]
Expand All @@ -102,10 +102,9 @@ export const expectExtensionError = (
stacktrace[0] = `Error: ${stacktrace[0]}`;

// We expect to see error details if:
// - includeStacktrace is true or
// - includeStacktrace is undefined and mode !== production
const expectErrors =
includeStacktrace === true || (includeStacktrace === undefined && mode !== 'production');
// - graphql.debug is true or
// - graphql.debug is undefined and mode !== production
const expectErrors = _debug === true || (_debug === undefined && mode !== 'production');
// We expect to see the Apollo exception under the same conditions, but only if
// httpQuery is also true.
const expectException = httpQuery && expectErrors;
Expand Down

0 comments on commit 208ae7a

Please sign in to comment.