Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Fix silent errors #476

Merged
merged 10 commits into from
Feb 28, 2017
24 changes: 23 additions & 1 deletion src/graphql.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,29 @@ export default function graphql(
// fetch the current result (if any) from the store
const currentResult = this.queryObservable.currentResult();
const { loading, error, networkStatus } = currentResult;
assign(data, { loading, error, networkStatus });
assign(data, { loading, networkStatus });

// Define the error property on the data object. If the user does
// not get the error object from `data` within 10 milliseconds
// then we will log the error to the console.
//
// 10 milliseconds is an arbitrary number picked to work around any
// potential asynchrony in React rendering. It is not super important
// that the error be logged ASAP, but 10 ms is enough to make it
// _feel_ like it was logged ASAP while still tolerating asynchrony.
let logErrorTimeoutId = setTimeout(() => {
if (error) {
console.error('Uncaught (in react-apollo)', error.stack || error);
}
}, 10);
Object.defineProperty(data, 'error', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be any benefit in only doing this in development mode? Or is the overhead minimal enough that we should just always do it?

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’m for always logging because:

  1. The overhead is fairly small.
  2. In production a user should really be catching the error anyway.
  3. Some error tracking libraries may report console.error exceptions.

configurable: true,
enumerable: true,
get: () => {
clearTimeout(logErrorTimeoutId);
return error;
},
});

if (loading) {
// while loading, we should use any previous data we have
Expand Down
7 changes: 5 additions & 2 deletions src/test-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ export class MockNetworkInterface implements NetworkInterface {

constructor(...mockedResponses: MockedResponse[]) {
mockedResponses.forEach((mockedResponse) => {
if (!mockedResponse.result && !mockedResponse.error) {
throw new Error('Mocked response should contain either result or error.');
}
this.addMockedReponse(mockedResponse);
});
}
Expand All @@ -127,7 +130,7 @@ export class MockNetworkInterface implements NetworkInterface {

const key = requestToKey(parsedRequest);

if (!this.mockedResponsesByKey[key]) {
if (!this.mockedResponsesByKey[key] || this.mockedResponsesByKey[key].length === 0) {
throw new Error('No more mocked responses for the query: ' + print(request.query));
}

Expand Down Expand Up @@ -227,7 +230,7 @@ export class MockSubscriptionNetworkInterface extends MockNetworkInterface imple
function requestToKey(request: ParsedRequest): string {
const queryString = request.query && print(request.query);
return JSON.stringify({
variables: request.variables,
variables: request.variables || {},
debugName: request.debugName,
query: queryString,
});
Expand Down
162 changes: 134 additions & 28 deletions test/react-web/client/graphql/queries.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe('queries', () => {

let error = null;
try {
renderer.create(<ApolloProvider client={client}><Container frst={1} /></ApolloProvider>);
renderer.create(<ApolloProvider client={client}><Container first={1} /></ApolloProvider>);
} catch (e) { error = e; }

expect(error).toBeNull();
Expand Down Expand Up @@ -1227,6 +1227,7 @@ describe('queries', () => {
const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } };
const networkInterface = mockNetworkInterface({ request: { query }, result: { data } });
const client = new ApolloClient({ networkInterface, addTypename: false });
let wrapper;

// @graphql(query)
@graphql(query, { options: { pollInterval: 10 }})
Expand All @@ -1236,14 +1237,17 @@ describe('queries', () => {
expect(data.startPolling instanceof Function).toBe(true);
// XXX this does throw because of no pollInterval
// expect(data.startPolling).not.toThrow();
done();
setTimeout(() => {
wrapper.unmount();
done();
}, 0);
}
render() {
return null;
}
};

renderer.create(<ApolloProvider client={client}><Container /></ApolloProvider>);
wrapper = renderer.create(<ApolloProvider client={client}><Container /></ApolloProvider>);
});

it('exposes networkStatus as a part of the props api', (done) => {
Expand Down Expand Up @@ -1384,7 +1388,7 @@ describe('queries', () => {
);
});

it('resets the loading state after a refetched query', (done) => {
it('resets the loading state after a refetched query', () => new Promise((resolve, reject) => {
const query = gql`query people { allPeople(first: 1) { people { name } } }`;
const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } };
const data2 = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } };
Expand All @@ -1394,39 +1398,38 @@ describe('queries', () => {
);
const client = new ApolloClient({ networkInterface, addTypename: false });

let refetched;
let isRefetching;
let count = 0;
@graphql(query, { options: { notifyOnNetworkStatusChange: true } })
class Container extends React.Component<any, any> {
componentWillReceiveProps = wrap(done, (props) => {
if (!isRefetching) {
isRefetching = true;
expect(props.data.networkStatus).toBe(7);
props.data.refetch();
return;
}

// get new data with no more loading state
if (isRefetching) {
expect(props.data.loading).toBe(false);
expect(props.data.networkStatus).toBe(4);
expect(props.data.allPeople).toEqual(data2.allPeople);
refetched = true;
return;
componentWillReceiveProps = wrap(reject, (props) => {
switch (count++) {
case 0:
expect(props.data.networkStatus).toBe(7);
props.data.refetch();
break;
case 1:
expect(props.data.loading).toBe(true);
expect(props.data.networkStatus).toBe(4);
expect(props.data.allPeople).toEqual(data.allPeople);
break;
case 2:
expect(props.data.loading).toBe(false);
expect(props.data.networkStatus).toBe(7);
expect(props.data.allPeople).toEqual(data2.allPeople);
resolve();
break;
default:
reject(new Error('Too many props updates'));
}
});

if (refetched) {
expect(props.data.networkStatus).toBe(7);
done();
}
})
render() {
return null;
}
};

renderer.create(<ApolloProvider client={client}><Container /></ApolloProvider>);
});
const wrapper = renderer.create(<ApolloProvider client={client}><Container /></ApolloProvider>);
}));

// XXX: this does not occur at the moment. When we add networkStatus, we should
// see a few more states
Expand Down Expand Up @@ -2315,4 +2318,107 @@ describe('queries', () => {
}, 10);
});

it('will not log a warning when there is an error that is caught in the render method', () => new Promise((resolve, reject) => {
const query = gql`query people { allPeople(first: 1) { people { name } } }`;
const networkInterface = mockNetworkInterface({ request: { query }, error: new Error('oops') });
const client = new ApolloClient({ networkInterface, addTypename: false });

const origError = console.error;
const errorMock = jest.fn();
console.error = errorMock;

let renderCount = 0;
@graphql(query)
class HandledErrorComponent extends React.Component<any, any> {
render() {
try {
switch (renderCount++) {
case 0:
expect(this.props.data.loading).toEqual(true);
break;
case 1:
expect(this.props.data.error.message).toEqual('Network error: oops');
break;
default:
throw new Error('Too many renders.');
}
} catch (error) {
console.error = origError;
reject(error);
}
return null;
}
}

renderer.create(
<ApolloProvider client={client}>
<HandledErrorComponent/>
</ApolloProvider>
);

setTimeout(() => {
try {
expect(renderCount).toBe(2);
expect(errorMock.mock.calls.length).toBe(0);
resolve();
} catch (error) {
reject(error);
} finally {
console.error = origError;
}
}, 20);
}));

it('will log a warning when there is an error that is not caught in the render method', () => new Promise((resolve, reject) => {
const query = gql`query people { allPeople(first: 1) { people { name } } }`;
const networkInterface = mockNetworkInterface({ request: { query }, error: new Error('oops') });
const client = new ApolloClient({ networkInterface, addTypename: false });

const origError = console.error;
const errorMock = jest.fn();
console.error = errorMock;

let renderCount = 0;
@graphql(query)
class UnhandledErrorComponent extends React.Component<any, any> {
render() {
try {
switch (renderCount++) {
case 0:
expect(this.props.data.loading).toEqual(true);
break;
case 1:
// Noop. Don’t handle the error so a warning will be logged to the console.
break;
default:
throw new Error('Too many renders.');
}
} catch (error) {
console.error = origError;
reject(error);
}
return null;
}
}

renderer.create(
<ApolloProvider client={client}>
<UnhandledErrorComponent/>
</ApolloProvider>
);

setTimeout(() => {
try {
expect(renderCount).toBe(2);
expect(errorMock.mock.calls.length).toBe(1);
expect(errorMock.mock.calls[0][0]).toEqual('Uncaught (in react-apollo)');
resolve();
} catch (error) {
reject(error);
} finally {
console.error = origError;
}
}, 20);
}));

});
58 changes: 38 additions & 20 deletions test/react-web/client/libraries/mobx.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,12 @@ describe('mobx integration', () => {
class AppState {
@observable first = 0;

constructor() {
setInterval(() => {
if (this.first <= 2) this.first += 1;
}, 250);
}

reset() {
this.first = 0;
}
}

it('works with mobx', (done) => {
it('works with mobx', () => new Promise((resolve, reject) => {
const query = gql`query people($first: Int) { allPeople(first: $first) { people { name } } }`;
const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } };
const variables = { first: 0 };
Expand All @@ -43,23 +37,43 @@ describe('mobx integration', () => {

const client = new ApolloClient({ networkInterface, addTypename: false });

let count = 0;

@graphql(query, {
options: (props) => ({ variables: { first: props.appState.first } }),
})
@observer
class Container extends React.Component<any, any> {
componentWillReact() {
if (this.props.appState.first === 1) {
this.props.data.refetch({ first: this.props.appState.first }).catch((e) => {
console.error(e);
});
}
}
componentWillReceiveProps(nextProps) {
if (this.props.appState.first === 1) {
if (nextProps.data.loading) return;
expect(nextProps.data.allPeople).toEqual(data2.allPeople);
done();
componentDidUpdate() {
try {
switch (count++) {
case 0:
expect(this.props.appState.first).toEqual(0);
expect(this.props.data.loading).toEqual(false);
expect(this.props.data.allPeople).toEqual(data.allPeople);
break;
case 1:
expect(this.props.appState.first).toEqual(1);
expect(this.props.data.loading).toEqual(false);
expect(this.props.data.allPeople).toEqual(data.allPeople);
this.props.data.refetch({ first: this.props.appState.first }).catch(reject);
break;
case 2:
expect(this.props.appState.first).toEqual(1);
expect(this.props.data.loading).toEqual(true);
expect(this.props.data.allPeople).toEqual(data.allPeople);
break;
case 3:
expect(this.props.appState.first).toEqual(1);
expect(this.props.data.loading).toEqual(false);
expect(this.props.data.allPeople).toEqual(data2.allPeople);
resolve();
break;
default:
throw new Error('Component updated to many times.');
}
} catch (error) {
reject(error);
}
}

Expand All @@ -69,12 +83,16 @@ describe('mobx integration', () => {
};

const appState = new AppState();

mount(
<ApolloProvider client={client}>
<Container appState={appState} />
</ApolloProvider>
) as any;

});
setTimeout(() => {
appState.first += 1;
}, 10);
}));

});