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

pass along missing field errors to the user #8262

Merged
merged 3 commits into from
May 20, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
- Fully remove result cache entries from LRU dependency system when the corresponding entities are removed from `InMemoryCache` by eviction, or by any other means. <br/>
[@sofianhn](https://github.com/sofianhn) and [@benjamn](https://github.com/benjamn) in [#8147](https://github.com/apollographql/apollo-client/pull/8147)

- Expose missing field errors in results. <br/> [@brainkim](github.com/brainkim) in [#8262](https://github.com/apollographql/apollo-client/pull/8262)

### Documentation
TBD

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.cjs.min.js",
"maxSize": "26.6 kB"
"maxSize": "26.62 kB"
}
],
"peerDependencies": {
Expand Down
9 changes: 7 additions & 2 deletions src/cache/core/types/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,19 @@ import { StorageType } from '../../inmemory/policies';
// Readonly<any>, somewhat surprisingly.
export type SafeReadonly<T> = T extends object ? Readonly<T> : T;

export class MissingFieldError {
export class MissingFieldError extends Error {
benjamn marked this conversation as resolved.
Show resolved Hide resolved
constructor(
public readonly message: string,
public readonly path: (string | number)[],
public readonly query: import('graphql').DocumentNode,
public readonly clientOnly: boolean,
public readonly variables?: Record<string, any>,
benjamn marked this conversation as resolved.
Show resolved Hide resolved
) {}
) {
super(message);
// We're not using `Object.setPrototypeOf` here as it isn't fully
// supported on Android (see issue #3236).
(this as any).__proto__ = MissingFieldError.prototype;
}
}

export interface FieldSpecifier {
Expand Down
11 changes: 11 additions & 0 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export class ObservableQuery<
!this.queryManager.transform(this.options.query).hasForcedResolvers
) {
const diff = this.queryInfo.getDiff();
// XXX the only reason this typechecks is that diff.result is inferred as any
result.data = (
diff.complete ||
this.options.returnPartialData
Expand All @@ -180,6 +181,16 @@ export class ObservableQuery<
} else {
result.partial = true;
}

if (
!diff.complete &&
!this.options.partialRefetch &&
!result.loading &&
!result.data &&
!result.error
) {
result.error = new ApolloError({ clientErrors: diff.missing });
}
}

if (saveAsLastResult) {
Expand Down
14 changes: 10 additions & 4 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ export function isApolloError(err: Error): err is ApolloError {
const generateErrorMessage = (err: ApolloError) => {
let message = '';
// If we have GraphQL errors present, add that to the error message.
if (isNonEmptyArray(err.graphQLErrors)) {
err.graphQLErrors.forEach((graphQLError: GraphQLError) => {
const errorMessage = graphQLError
? graphQLError.message
if (isNonEmptyArray(err.graphQLErrors) || isNonEmptyArray(err.clientErrors)) {
const errors = ((err.graphQLErrors || []) as readonly Error[])
.concat(err.clientErrors || []);
errors.forEach((error: Error) => {
const errorMessage = error
? error.message
: 'Error message not found.';
message += `${errorMessage}\n`;
});
Expand All @@ -36,6 +38,7 @@ const generateErrorMessage = (err: ApolloError) => {
export class ApolloError extends Error {
public message: string;
public graphQLErrors: ReadonlyArray<GraphQLError>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I consider Readonly to be a “bad part” of TypeScript but I’m not gonna rock the boat here.

public clientErrors: ReadonlyArray<Error>;
public networkError: Error | ServerParseError | ServerError | null;

// An object that can be used to provide some additional information
Expand All @@ -48,17 +51,20 @@ export class ApolloError extends Error {
// value or the constructed error will be meaningless.
constructor({
graphQLErrors,
clientErrors,
networkError,
errorMessage,
extraInfo,
}: {
graphQLErrors?: ReadonlyArray<GraphQLError>;
clientErrors?: ReadonlyArray<Error>;
networkError?: Error | ServerParseError | ServerError | null;
errorMessage?: string;
extraInfo?: any;
}) {
super(errorMessage);
this.graphQLErrors = graphQLErrors || [];
this.clientErrors = clientErrors || [];
this.networkError = networkError || null;
this.message = errorMessage || generateErrorMessage(this);
this.extraInfo = extraInfo;
Expand Down
77 changes: 77 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2645,6 +2645,83 @@ describe('useQuery Hook', () => {
});
});

describe('Missing Fields', () => {
itAsync(
'should have errors populated with missing field errors from the cache',
(resolve, reject) => {
const carQuery: DocumentNode = gql`
query cars($id: Int) {
cars(id: $id) {
id
make
model
vin
__typename
}
}
`;

const carData = {
cars: [
{
id: 1,
make: 'Audi',
model: 'RS8',
vine: 'DOLLADOLLABILL',
__typename: 'Car'
}
]
};

const mocks = [
{
request: { query: carQuery, variables: { id: 1 } },
result: { data: carData }
},
];

let renderCount = 0;
function App() {
const { loading, data, error } = useQuery(carQuery, {
variables: { id: 1 },
});

switch (renderCount) {
case 0:
expect(loading).toBeTruthy();
expect(data).toBeUndefined();
expect(error).toBeUndefined();
break;
case 1:
expect(loading).toBeFalsy();
expect(data).toBeUndefined();
expect(error).toBeDefined();
// TODO: ApolloError.name is Error for some reason
// expect(error!.name).toBe(ApolloError);
expect(error!.clientErrors.length).toEqual(1);
expect(error!.message).toMatch(/Can't find field 'vin' on Car:1/);
break;
default:
throw new Error("Unexpected render");
}

renderCount += 1;
return null;
}

render(
<MockedProvider mocks={mocks}>
<App />
</MockedProvider>
);

return wait(() => {
expect(renderCount).toBe(2);
}).then(resolve, reject);
},
);
});

describe('Previous data', () => {
itAsync('should persist previous data when a query is re-run', (resolve, reject) => {
const query = gql`
Expand Down