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

refactor: use object for GraphQLError constructor #3465

Merged
merged 2 commits into from Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
123 changes: 46 additions & 77 deletions src/error/__tests__/GraphQLError-test.ts
Expand Up @@ -40,15 +40,14 @@ describe('GraphQLError', () => {
});

it('enumerate only properties prescribed by the spec', () => {
const e = new GraphQLError(
'msg' /* message */,
[fieldNode] /* nodes */,
source /* source */,
[1, 2, 3] /* positions */,
['a', 'b', 'c'] /* path */,
new Error('test') /* originalError */,
{ foo: 'bar' } /* extensions */,
);
const e = new GraphQLError('msg' /* message */, {
nodes: [fieldNode],
source,
positions: [1, 2, 3],
path: ['a', 'b', 'c'],
originalError: new Error('test'),
extensions: { foo: 'bar' },
});

expect(Object.keys(e)).to.deep.equal([
'message',
Expand All @@ -60,14 +59,9 @@ describe('GraphQLError', () => {

it('uses the stack of an original error', () => {
const original = new Error('original');
const e = new GraphQLError(
'msg',
undefined,
undefined,
undefined,
undefined,
original,
);
const e = new GraphQLError('msg', {
originalError: original,
});

expect(e).to.include({
name: 'GraphQLError',
Expand All @@ -79,7 +73,7 @@ describe('GraphQLError', () => {

it('creates new stack if original error has no stack', () => {
const original = new Error('original');
const e = new GraphQLError('msg', null, null, null, null, original);
const e = new GraphQLError('msg', { originalError: original });

expect(e).to.include({
name: 'GraphQLError',
Expand All @@ -90,7 +84,7 @@ describe('GraphQLError', () => {
});

it('converts nodes to positions and locations', () => {
const e = new GraphQLError('msg', [fieldNode]);
const e = new GraphQLError('msg', { nodes: [fieldNode] });
expect(e).to.deep.include({
source,
nodes: [fieldNode],
Expand All @@ -100,7 +94,7 @@ describe('GraphQLError', () => {
});

it('converts single node to positions and locations', () => {
const e = new GraphQLError('msg', fieldNode); // Non-array value.
const e = new GraphQLError('msg', { nodes: fieldNode }); // Non-array value.
expect(e).to.deep.include({
source,
nodes: [fieldNode],
Expand All @@ -110,7 +104,7 @@ describe('GraphQLError', () => {
});

it('converts node with loc.start === 0 to positions and locations', () => {
const e = new GraphQLError('msg', operationNode);
const e = new GraphQLError('msg', { nodes: operationNode });
expect(e).to.deep.include({
source,
nodes: [operationNode],
Expand All @@ -125,7 +119,7 @@ describe('GraphQLError', () => {
loc: undefined,
};

const e = new GraphQLError('msg', fieldNodeNoLocation);
const e = new GraphQLError('msg', { nodes: fieldNodeNoLocation });
expect(e).to.deep.include({
nodes: [fieldNodeNoLocation],
source: undefined,
Expand All @@ -135,7 +129,7 @@ describe('GraphQLError', () => {
});

it('converts source and positions to locations', () => {
const e = new GraphQLError('msg', null, source, [6]);
const e = new GraphQLError('msg', { source, positions: [6] });
expect(e).to.deep.include({
source,
nodes: undefined,
Expand All @@ -155,47 +149,31 @@ describe('GraphQLError', () => {
}

const original = new ErrorWithExtensions('original');
const inheritedExtensions = new GraphQLError(
'InheritedExtensions',
undefined,
undefined,
undefined,
undefined,
original,
undefined,
);
const inheritedExtensions = new GraphQLError('InheritedExtensions', {
originalError: original,
});

expect(inheritedExtensions).to.deep.include({
message: 'InheritedExtensions',
originalError: original,
extensions: { original: 'extensions' },
});

const ownExtensions = new GraphQLError(
'OwnExtensions',
undefined,
undefined,
undefined,
undefined,
original,
{ own: 'extensions' },
);
const ownExtensions = new GraphQLError('OwnExtensions', {
originalError: original,
extensions: { own: 'extensions' },
});

expect(ownExtensions).to.deep.include({
message: 'OwnExtensions',
originalError: original,
extensions: { own: 'extensions' },
});

const ownEmptyExtensions = new GraphQLError(
'OwnEmptyExtensions',
undefined,
undefined,
undefined,
undefined,
original,
{},
);
const ownEmptyExtensions = new GraphQLError('OwnEmptyExtensions', {
originalError: original,
extensions: {},
});

expect(ownEmptyExtensions).to.deep.include({
message: 'OwnEmptyExtensions',
Expand All @@ -214,15 +192,11 @@ describe('GraphQLError', () => {

const path = ['path', 2, 'field'];
const extensions = { foo: 'bar' };
const eFull = new GraphQLError(
'msg',
fieldNode,
undefined,
undefined,
const eFull = new GraphQLError('msg', {
nodes: fieldNode,
path,
undefined,
extensions,
);
});

// We should try to keep order of fields stable
// Changing it wouldn't be breaking change but will fail some tests in other libraries.
Expand Down Expand Up @@ -260,10 +234,9 @@ describe('toString', () => {
});

it('prints an error using node without location', () => {
const error = new GraphQLError(
'Error attached to node without location',
parse('{ foo }', { noLocation: true }),
);
const error = new GraphQLError('Error attached to node without location', {
nodes: parse('{ foo }', { noLocation: true }),
});
expect(error.toString()).to.equal(
'Error attached to node without location',
);
Expand Down Expand Up @@ -330,12 +303,7 @@ describe('toJSON', () => {
});

it('includes path', () => {
const error = new GraphQLError('msg', null, null, null, [
'path',
3,
'to',
'field',
]);
const error = new GraphQLError('msg', { path: ['path', 3, 'to', 'field'] });

expect(error.toJSON()).to.deep.equal({
message: 'msg',
Expand All @@ -344,8 +312,8 @@ describe('toJSON', () => {
});

it('includes extension fields', () => {
const error = new GraphQLError('msg', null, null, null, null, null, {
foo: 'bar',
const error = new GraphQLError('msg', {
extensions: { foo: 'bar' },
});

expect(error.toJSON()).to.deep.equal({
Expand All @@ -354,15 +322,16 @@ describe('toJSON', () => {
});
});

it('can be created with the alternative object argument', () => {
const error = new GraphQLError('msg', {
nodes: [operationNode],
it('can be created with the legacy argument list', () => {
const error = new GraphQLError(
'msg',
[operationNode],
source,
positions: [6],
path: ['path', 2, 'a'],
originalError: new Error('I like turtles'),
extensions: { hee: 'I like turtles' },
});
[6],
['path', 2, 'a'],
new Error('I like turtles'),
{ hee: 'I like turtles' },
);

expect(error.toJSON()).to.deep.equal({
message: 'msg',
Expand Down
7 changes: 1 addition & 6 deletions src/error/__tests__/locatedError-test.ts
Expand Up @@ -6,12 +6,7 @@ import { locatedError } from '../locatedError';

describe('locatedError', () => {
it('passes GraphQLError through', () => {
const e = new GraphQLError('msg', null, null, null, [
'path',
3,
'to',
'field',
]);
const e = new GraphQLError('msg', { path: ['path', 3, 'to', 'field'] });

expect(locatedError(e, [], [])).to.deep.equal(e);
});
Expand Down
11 changes: 5 additions & 6 deletions src/error/locatedError.ts
Expand Up @@ -22,14 +22,13 @@ export function locatedError(
return originalError;
}

return new GraphQLError(
originalError.message,
(originalError as GraphQLError).nodes ?? nodes,
(originalError as GraphQLError).source,
(originalError as GraphQLError).positions,
return new GraphQLError(originalError.message, {
nodes: (originalError as GraphQLError).nodes ?? nodes,
source: (originalError as GraphQLError).source,
positions: (originalError as GraphQLError).positions,
path,
originalError,
);
});
}

function isLocatedGraphQLError(error: any): error is GraphQLError {
Expand Down
7 changes: 4 additions & 3 deletions src/error/syntaxError.ts
Expand Up @@ -11,7 +11,8 @@ export function syntaxError(
position: number,
description: string,
): GraphQLError {
return new GraphQLError(`Syntax Error: ${description}`, undefined, source, [
position,
]);
return new GraphQLError(`Syntax Error: ${description}`, {
source,
positions: [position],
});
}
10 changes: 5 additions & 5 deletions src/execution/execute.ts
Expand Up @@ -362,7 +362,7 @@ function executeOperation(
if (rootType == null) {
throw new GraphQLError(
`Schema is not configured to execute ${operation.operation} operation.`,
operation,
{ nodes: operation },
);
}

Expand Down Expand Up @@ -881,21 +881,21 @@ function ensureValidRuntimeType(
if (runtimeType == null) {
throw new GraphQLError(
`Abstract type "${returnType.name}" was resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`,
fieldNodes,
{ nodes: fieldNodes },
);
}

if (!isObjectType(runtimeType)) {
throw new GraphQLError(
`Abstract type "${returnType.name}" was resolved to a non-object type "${runtimeTypeName}".`,
fieldNodes,
{ nodes: fieldNodes },
);
}

if (!exeContext.schema.isSubType(returnType, runtimeType)) {
throw new GraphQLError(
`Runtime Object type "${runtimeType.name}" is not a possible type for "${returnType.name}".`,
fieldNodes,
{ nodes: fieldNodes },
);
}

Expand Down Expand Up @@ -952,7 +952,7 @@ function invalidReturnTypeError(
): GraphQLError {
return new GraphQLError(
`Expected value of type "${returnType.name}" but got: ${inspect(result)}.`,
fieldNodes,
{ nodes: fieldNodes },
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/execution/subscribe.ts
Expand Up @@ -194,7 +194,7 @@ async function executeSubscription(
if (rootType == null) {
throw new GraphQLError(
'Schema is not configured to execute subscription operation.',
operation,
{ nodes: operation },
);
}

Expand All @@ -212,7 +212,7 @@ async function executeSubscription(
const fieldName = fieldNodes[0].name.value;
throw new GraphQLError(
`The subscription field "${fieldName}" is not defined.`,
fieldNodes,
{ nodes: fieldNodes },
);
}

Expand Down