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

Safe Apollo Reducers that can't crash the store #874

Merged
merged 7 commits into from
Dec 1, 2016
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 3 to 6 months), to signal the start of a more stable API.

### vNEXT
- Prevent Redux from crashing when an uncaught ApolloError is raised in an Apollo reducer. [PR #874](https://github.com/apollostack/apollo-client/pull/874)

### 0.5.8

Expand Down
6 changes: 6 additions & 0 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -965,9 +965,15 @@ export class QueryManager {
} catch (e) {}
/* tslint:enable */

const {reducerError} = this.getApolloState();
if (!resultFromStore && reducerError) {
return Promise.reject(reducerError);
}

// return a chainable promise
this.removeFetchQueryPromise(requestId);
resolve({ data: resultFromStore, loading: false, networkStatus: NetworkStatus.ready });
return null;
}).catch((error: Error) => {
// This is for the benefit of `refetch` promises, which currently don't get their errors
// through the store like watchQuery observers do
Expand Down
61 changes: 35 additions & 26 deletions src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface Store {
queries: QueryStore;
mutations: MutationStore;
optimistic: OptimisticStore;
reducerError: Error | null;
}

/**
Expand Down Expand Up @@ -79,32 +80,40 @@ export type ApolloReducer = (store: NormalizedCache, action: ApolloAction) => No

export function createApolloReducer(config: ApolloReducerConfig): Function {
return function apolloReducer(state = {} as Store, action: ApolloAction) {
const newState = {
queries: queries(state.queries, action),
mutations: mutations(state.mutations, action),

// Note that we are passing the queries into this, because it reads them to associate
// the query ID in the result with the actual query
data: data(state.data, action, state.queries, state.mutations, config),
optimistic: [] as any,
};

// use the two lines below to debug tests :)
// console.log('ACTION', action.type);
// console.log('new state', newState);

// Note, we need to have the results of the
// APOLLO_MUTATION_INIT action to simulate
// the APOLLO_MUTATION_RESULT action. That's
// why we pass in newState
newState.optimistic = optimistic(
state.optimistic,
action,
newState,
config
);

return newState;
try {
const newState: Store = {
queries: queries(state.queries, action),
mutations: mutations(state.mutations, action),

// Note that we are passing the queries into this, because it reads them to associate
// the query ID in the result with the actual query
data: data(state.data, action, state.queries, state.mutations, config),
optimistic: [] as any,

reducerError: null,
};

// use the two lines below to debug tests :)
// console.log('ACTION', action.type);
// console.log('new state', newState);

// Note, we need to have the results of the
// APOLLO_MUTATION_INIT action to simulate
// the APOLLO_MUTATION_RESULT action. That's
// why we pass in newState
newState.optimistic = optimistic(
state.optimistic,
action,
newState,
config
);

return newState;
} catch (reducerError) {
return Object.assign({}, state, {
reducerError,
});
}
};
}

Expand Down
1 change: 1 addition & 0 deletions test/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,7 @@ describe('QueryManager', () => {
mutations: {},
queries: {},
optimistic: [],
reducerError: null,
};

assert.deepEqual(currentState, expectedState);
Expand Down
2 changes: 2 additions & 0 deletions test/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ describe('client', () => {
mutations: {},
data: {},
optimistic: [],
reducerError: null,
},
}
);
Expand Down Expand Up @@ -498,6 +499,7 @@ describe('client', () => {
},
},
mutations: {},
reducerError: null,
}) };

const client = new ApolloClient({
Expand Down
73 changes: 71 additions & 2 deletions test/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { assert } = chai;
import gql from 'graphql-tag';

import {
Store,
createApolloStore,
} from '../src/store';

Expand All @@ -21,6 +22,7 @@ describe('createApolloStore', () => {
mutations: {},
data: {},
optimistic: [],
reducerError: null,
}
);
});
Expand All @@ -37,6 +39,7 @@ describe('createApolloStore', () => {
mutations: {},
data: {},
optimistic: [],
reducerError: null,
}
);
});
Expand All @@ -61,6 +64,7 @@ describe('createApolloStore', () => {
mutations: {},
data: initialState.apollo.data,
optimistic: initialState.apollo.optimistic,
reducerError: null,
},
});
});
Expand Down Expand Up @@ -110,11 +114,12 @@ describe('createApolloStore', () => {
},
};

const emptyState = {
const emptyState: Store = {
queries: { },
mutations: { },
data: { },
optimistic: ([] as any[]),
reducerError: null,
};

const store = createApolloStore({
Expand All @@ -140,7 +145,7 @@ describe('createApolloStore', () => {
},
};

const emptyState = {
const emptyState: Store = {
queries: {
'test.0': {
'forceFetch': false,
Expand All @@ -159,6 +164,7 @@ describe('createApolloStore', () => {
mutations: {},
data: {},
optimistic: ([] as any[]),
reducerError: null,
};

const store = createApolloStore({
Expand Down Expand Up @@ -186,4 +192,67 @@ describe('createApolloStore', () => {

assert.deepEqual(store.getState().apollo, emptyState);
});

it('can\'t crash the reducer', () => {
const initialState = {
apollo: {
data: {},
optimistic: ([] as any[]),
reducerError: (null as Error | null),
},
};

const store = createApolloStore({
initialState,
});

// Try to crash the store with a bad behavior update
const mutationString = `mutation Increment { incrementer { counter } }`;
const mutation = gql`${mutationString}`;

store.dispatch({
type: 'APOLLO_MUTATION_INIT',
mutationString,
mutation,
variables: {},
operationName: 'Increment',
mutationId: '1',
optimisticResponse: {data: {incrementer: {counter: 1}}},
});
const throwingBehavior: any = [
{
type: 'UnknownBehavior',
},
];
store.dispatch({
type: 'APOLLO_MUTATION_RESULT',
result: {data: {incrementer: {counter: 1}}},
document: mutation,
operationName: 'Increment',
mutationId: '1',
resultBehaviors: throwingBehavior,
});

assert(/UnknownBehavior/.test(store.getState().apollo.reducerError));

const resetState = {
queries: {},
mutations: {},
data: {},
optimistic: [
{
data: {},
mutationId: '1',
},
],
reducerError: (null as Error | null),
};

store.dispatch({
type: 'APOLLO_STORE_RESET',
observableQueryIds: ['test.0'],
});

assert.deepEqual(store.getState().apollo, resetState);
});
});