Skip to content

Commit

Permalink
Remove stray it.only from QueryManager tests (and fix tests).
Browse files Browse the repository at this point in the history
Thanks to @hwillson for catching this!
#4743 (comment)
  • Loading branch information
benjamn committed Apr 23, 2019
1 parent 0965326 commit b095595
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 52 deletions.
24 changes: 18 additions & 6 deletions packages/apollo-client/src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { QueryStoreValue } from '../data/queries';

import { invariant, InvariantError } from 'ts-invariant';
import { isNonEmptyArray } from '../util/arrays';

// XXX remove in the next breaking semver change (3.0)
// Deprecated, use ApolloCurrentQueryResult
Expand Down Expand Up @@ -63,12 +64,10 @@ export interface UpdateQueryOptions<TVariables> {
export const hasError = (
storeValue: QueryStoreValue,
policy: ErrorPolicy = 'none',
) =>
storeValue &&
((storeValue.graphQLErrors &&
storeValue.graphQLErrors.length > 0 &&
policy === 'none') ||
storeValue.networkError);
) => storeValue && (
storeValue.networkError ||
(policy === 'none' && isNonEmptyArray(storeValue.graphQLErrors))
);

export class ObservableQuery<
TData = any,
Expand Down Expand Up @@ -567,6 +566,15 @@ export class ObservableQuery<
}

private onSubscribe(observer: Observer<ApolloQueryResult<TData>>) {
// Zen Observable has its own error function, so in order to log correctly
// we need to provide a custom error callback.
try {
var subObserver = (observer as any)._subscription._observer;
if (subObserver && !subObserver.error) {
subObserver.error = defaultSubscriptionObserverErrorCallback;
}
} catch {}

const first = !this.observers.size;
this.observers.add(observer);

Expand Down Expand Up @@ -659,6 +667,10 @@ export class ObservableQuery<
}
}

function defaultSubscriptionObserverErrorCallback(error: ApolloError) {
invariant.error('Unhandled error', error.message, error.stack);
}

function iterateObserversSafely<E, A>(
observers: Set<Observer<E>>,
method: keyof Observer<E>,
Expand Down
62 changes: 30 additions & 32 deletions packages/apollo-client/src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
} from './types';
import { LocalState } from './LocalState';
import { asyncMap, multiplex } from '../util/observables';
import { isNonEmptyArray } from '../util/arrays';

const { hasOwnProperty } = Object.prototype;

Expand Down Expand Up @@ -285,30 +286,32 @@ export class QueryManager<TStore> {
ApolloQueryResult<any>[] | ApolloQueryResult<{}>
>[] = [];

refetchQueries.forEach(refetchQuery => {
if (typeof refetchQuery === 'string') {
self.queries.forEach(({ observableQuery }) => {
if (
observableQuery &&
observableQuery.queryName === refetchQuery
) {
refetchQueryPromises.push(observableQuery.refetch());
if (isNonEmptyArray(refetchQueries)) {
refetchQueries.forEach(refetchQuery => {
if (typeof refetchQuery === 'string') {
self.queries.forEach(({ observableQuery }) => {
if (
observableQuery &&
observableQuery.queryName === refetchQuery
) {
refetchQueryPromises.push(observableQuery.refetch());
}
});
} else {
const queryOptions: QueryOptions = {
query: refetchQuery.query,
variables: refetchQuery.variables,
fetchPolicy: 'network-only',
};

if (refetchQuery.context) {
queryOptions.context = refetchQuery.context;
}
});
} else {
const queryOptions: QueryOptions = {
query: refetchQuery.query,
variables: refetchQuery.variables,
fetchPolicy: 'network-only',
};

if (refetchQuery.context) {
queryOptions.context = refetchQuery.context;
refetchQueryPromises.push(self.query(queryOptions));
}

refetchQueryPromises.push(self.query(queryOptions));
}
});
});
}

Promise.all(
awaitRefetchQueries ? refetchQueryPromises : [],
Expand Down Expand Up @@ -558,9 +561,7 @@ export class QueryManager<TStore> {
return;
}

const hasGraphQLErrors =
queryStoreValue.graphQLErrors &&
queryStoreValue.graphQLErrors.length > 0;
const hasGraphQLErrors = isNonEmptyArray(queryStoreValue.graphQLErrors);

const errorPolicy: ErrorPolicy = observableQuery
&& observableQuery.options.errorPolicy
Expand Down Expand Up @@ -1232,10 +1233,10 @@ export class QueryManager<TStore> {
this.broadcastQueries();
}

if (result.errors && errorPolicy === 'none') {
throw new ApolloError({
if (errorPolicy === 'none' && isNonEmptyArray(result.errors)) {
return reject(new ApolloError({
graphQLErrors: result.errors,
});
}));
}

if (errorPolicy === 'all') {
Expand All @@ -1259,16 +1260,13 @@ export class QueryManager<TStore> {
// tslint:disable-next-line
} catch (e) {}
}

return resultFromStore;

}).subscribe({
error: (error: ApolloError) => {
error(error: ApolloError) {
cleanup();
reject(error);
},

complete: () => {
complete() {
cleanup();
resolve({
data: resultFromStore,
Expand Down
24 changes: 13 additions & 11 deletions packages/apollo-client/src/core/__tests__/QueryManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1626,7 +1626,7 @@ describe('QueryManager', () => {
queryManager.mutate({
// Bamboozle TypeScript into letting us do this
mutation: ('string' as any) as DocumentNode,
})
}),
).rejects.toThrow(/wrap the query string in a "gql" tag/);

expect(() => {
Expand Down Expand Up @@ -2315,10 +2315,17 @@ describe('QueryManager', () => {

const handle = queryManager.watchQuery<any>(request);

const checkError = error => {
expect(error.graphQLErrors).toEqual([
{
name: 'PeopleError',
message: 'This is not the person you are looking for.',
},
]);
};

handle.subscribe({
error: () => {
/* nothing */
},
error: checkError,
});

handle
Expand All @@ -2327,12 +2334,7 @@ describe('QueryManager', () => {
done.fail(new Error('Error on refetch should reject promise'));
})
.catch(error => {
expect(error.graphQLErrors).toEqual([
{
name: 'PeopleError',
message: 'This is not the person you are looking for.',
},
]);
checkError(error);
done();
});

Expand Down Expand Up @@ -3526,7 +3528,7 @@ describe('QueryManager', () => {
});
});

it.only('should only refetch once when we refetch observable queries', (done) => {
it('should only refetch once when we refetch observable queries', done => {
let queryManager: QueryManager<NormalizedCacheObject>;
const query = gql`
query {
Expand Down
4 changes: 2 additions & 2 deletions packages/apollo-client/src/data/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { DocumentNode, GraphQLError, ExecutionResult } from 'graphql';
import { isEqual } from 'apollo-utilities';
import { InvariantError } from 'ts-invariant';
import { NetworkStatus } from '../core/networkStatus';
import { isNonEmptyArray } from '../util/arrays';

export type QueryStoreValue = {
document: DocumentNode;
Expand Down Expand Up @@ -119,8 +120,7 @@ export class QueryStore {
if (!this.store || !this.store[queryId]) return;

this.store[queryId].networkError = null;
this.store[queryId].graphQLErrors =
result.errors && result.errors.length ? result.errors : [];
this.store[queryId].graphQLErrors = isNonEmptyArray(result.errors) ? result.errors : [];
this.store[queryId].previousVariables = null;
this.store[queryId].networkStatus = NetworkStatus.ready;

Expand Down
3 changes: 2 additions & 1 deletion packages/apollo-client/src/errors/ApolloError.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { GraphQLError } from 'graphql';
import { isNonEmptyArray } from '../util/arrays';

export function isApolloError(err: Error): err is ApolloError {
return err.hasOwnProperty('graphQLErrors');
Expand All @@ -11,7 +12,7 @@ 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 (Array.isArray(err.graphQLErrors) && err.graphQLErrors.length !== 0) {
if (isNonEmptyArray(err.graphQLErrors)) {
err.graphQLErrors.forEach((graphQLError: GraphQLError) => {
const errorMessage = graphQLError
? graphQLError.message
Expand Down
3 changes: 3 additions & 0 deletions packages/apollo-client/src/util/arrays.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isNonEmptyArray<T>(value?: ArrayLike<T>): value is Array<T> {
return Array.isArray(value) && value.length > 0;
}

0 comments on commit b095595

Please sign in to comment.