Skip to content

Commit e12ccdd

Browse files
trevorbladestrevorblades
authoredAug 27, 2019
Merge branch 'master' into renovate/documentation-theme
·
publish/20231114001151@apollo/cache-control-types@1.0.3
2 parents 11d2c56 + 4675c67 commit e12ccdd

File tree

3 files changed

+118
-60
lines changed

3 files changed

+118
-60
lines changed
 

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ The version headers in this history reflect the versions of Apollo Server itself
77
> The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.
88
99
- `apollo-server-core`: Make `formatError` available to subscriptions in the same spirit as the existing `formatResponse`. [PR #2942](https://github.com/apollographql/apollo-server/pull/2942)
10+
- `apollo-engine-reporting`: The behavior of the `engine.maxAttempts` parameter previously did not match its documentation. It is documented as being the max number of attempts *including* the initial attempt, but until this release it was actually the number of retries *excluding* the initial attempt. The behavior has been changed to match the documentation (and the literal reading of the option name). [PR #3218](https://github.com/apollographql/apollo-server/pull/3218)
11+
- `apollo-engine-reporting`: When sending the report fails with a server-side 5xx error, include the full error from the server in the logs. [PR #3218](https://github.com/apollographql/apollo-server/pull/3218)
1012

1113
### v2.9.0
1214

‎packages/apollo-engine-reporting/src/agent.ts

+10-5
Original file line numberDiff line numberDiff line change
@@ -403,27 +403,32 @@ export class EngineReportingAgent<TContext = any> {
403403
});
404404

405405
if (curResponse.status >= 500 && curResponse.status < 600) {
406-
throw new Error(`${curResponse.status}: ${curResponse.statusText}`);
406+
throw new Error(
407+
`HTTP status ${curResponse.status}, ${(await curResponse.text()) ||
408+
'(no body)'}`,
409+
);
407410
} else {
408411
return curResponse;
409412
}
410413
},
411414
{
412-
retries: this.options.maxAttempts || 5,
415+
retries: (this.options.maxAttempts || 5) - 1,
413416
minTimeout: this.options.minimumRetryDelayMs || 100,
414417
factor: 2,
415418
},
416419
).catch((err: Error) => {
417-
throw new Error(`Error sending report to Apollo Engine servers: ${err}`);
420+
throw new Error(
421+
`Error sending report to Apollo Engine servers: ${err.message}`,
422+
);
418423
});
419424

420425
if (response.status < 200 || response.status >= 300) {
421426
// Note that we don't expect to see a 3xx here because request follows
422427
// redirects.
423428
throw new Error(
424-
`Error sending report to Apollo Engine servers (HTTP status ${
429+
`Error sending report to Apollo Engine servers: HTTP status ${
425430
response.status
426-
}): ${await response.text()}`,
431+
}, ${(await response.text()) || '(no body)'}`,
427432
);
428433
}
429434
if (this.options.debugPrintReports) {

‎packages/apollo-server-integration-testsuite/src/ApolloServer.ts

+106-55
Original file line numberDiff line numberDiff line change
@@ -1945,70 +1945,121 @@ export function testApolloServer<AS extends ApolloServerBase>(
19451945
});
19461946

19471947
describe('apollo-engine-reporting', () => {
1948-
it('graphql server functions even when Apollo servers are down', async () => {
1949-
let return502Resolve: () => void;
1950-
const return502Promise = new Promise(
1951-
resolve => (return502Resolve = resolve),
1952-
);
1953-
const fakeEngineServer = http.createServer(async (_, res) => {
1954-
await return502Promise;
1955-
res.writeHead(502);
1956-
res.end();
1957-
});
1958-
await new Promise(resolve => {
1959-
fakeEngineServer.listen(0, '127.0.0.1', () => {
1960-
resolve();
1948+
describe('graphql server functions even when Apollo servers are down', () => {
1949+
async function testWithStatus(
1950+
status: number,
1951+
expectedRequestCount: number,
1952+
) {
1953+
const networkError = status === 0;
1954+
1955+
let writeResponseResolve: () => void;
1956+
const writeResponsePromise = new Promise(
1957+
resolve => (writeResponseResolve = resolve),
1958+
);
1959+
const fakeEngineServer = http.createServer(async (_, res) => {
1960+
await writeResponsePromise;
1961+
res.writeHead(status);
1962+
res.end('Important text in the body');
19611963
});
1962-
});
1963-
try {
1964-
const { family, address, port } = fakeEngineServer.address();
1965-
if (family !== 'IPv4') {
1966-
throw new Error(`The family was unexpectedly ${family}.`);
1964+
await new Promise(resolve => {
1965+
fakeEngineServer.listen(0, '127.0.0.1', () => {
1966+
resolve();
1967+
});
1968+
});
1969+
async function closeServer() {
1970+
await new Promise(resolve =>
1971+
fakeEngineServer.close(() => resolve()),
1972+
);
19671973
}
1968-
const fakeEngineUrl = new URL(`http://${address}:${port}`).toString();
1974+
try {
1975+
const { family, address, port } = fakeEngineServer.address();
1976+
if (family !== 'IPv4') {
1977+
throw new Error(`The family was unexpectedly ${family}.`);
1978+
}
1979+
const fakeEngineUrl = `http://${address}:${port}`;
19691980

1970-
let reportErrorPromiseResolve: (error: Error) => void;
1971-
const reportErrorPromise = new Promise<Error>(
1972-
resolve => (reportErrorPromiseResolve = resolve),
1973-
);
1974-
const { url: uri } = await createApolloServer({
1975-
typeDefs: gql`
1976-
type Query {
1977-
something: String!
1978-
}
1979-
`,
1980-
resolvers: { Query: { something: () => 'hello' } },
1981-
engine: {
1982-
apiKey: 'service:my-app:secret',
1983-
endpointUrl: fakeEngineUrl,
1984-
reportIntervalMs: 1,
1985-
maxAttempts: 1,
1986-
reportErrorFunction(error: Error) {
1987-
reportErrorPromiseResolve(error);
1981+
// To simulate a network error, we create and close the server.
1982+
// This lets us still generate a port that is hopefully unused.
1983+
if (networkError) {
1984+
await closeServer();
1985+
}
1986+
1987+
let requestCount = 0;
1988+
const requestAgent = new http.Agent({ keepAlive: false });
1989+
const realCreateConnection = (requestAgent as any).createConnection;
1990+
(requestAgent as any).createConnection = function() {
1991+
requestCount++;
1992+
return realCreateConnection.apply(this, arguments);
1993+
};
1994+
1995+
let reportErrorPromiseResolve: (error: Error) => void;
1996+
const reportErrorPromise = new Promise<Error>(
1997+
resolve => (reportErrorPromiseResolve = resolve),
1998+
);
1999+
const { url: uri } = await createApolloServer({
2000+
typeDefs: gql`
2001+
type Query {
2002+
something: String!
2003+
}
2004+
`,
2005+
resolvers: { Query: { something: () => 'hello' } },
2006+
engine: {
2007+
apiKey: 'service:my-app:secret',
2008+
endpointUrl: fakeEngineUrl,
2009+
reportIntervalMs: 1,
2010+
maxAttempts: 3,
2011+
requestAgent,
2012+
reportErrorFunction(error: Error) {
2013+
reportErrorPromiseResolve(error);
2014+
},
19882015
},
1989-
},
1990-
});
2016+
});
19912017

1992-
const apolloFetch = createApolloFetch({ uri });
2018+
const apolloFetch = createApolloFetch({ uri });
19932019

1994-
// Run a GraphQL query. Ensure that it returns successfully even
1995-
// though reporting is going to fail. (Note that reporting can't
1996-
// actually have failed yet because we haven't let return502Promise
1997-
// resolve.)
1998-
const result = await apolloFetch({
1999-
query: `{ something }`,
2000-
});
2001-
expect(result.data.something).toBe('hello');
2020+
// Run a GraphQL query. Ensure that it returns successfully even
2021+
// though reporting is going to fail. (Note that reporting can't
2022+
// actually have failed yet (except in the network-error case)
2023+
// because we haven't let writeResponsePromise resolve.)
2024+
const result = await apolloFetch({
2025+
query: `{ something }`,
2026+
});
2027+
expect(result.data.something).toBe('hello');
20022028

2003-
// Allow reporting to return 502.
2004-
return502Resolve();
2029+
if (!networkError) {
2030+
// Allow reporting to return its response (for every retry).
2031+
writeResponseResolve();
2032+
}
20052033

2006-
// Make sure we can get the 502 error from reporting.
2007-
const sendingError = await reportErrorPromise;
2008-
expect(sendingError.message).toContain('Error: 502: Bad Gateway');
2009-
} finally {
2010-
await new Promise(resolve => fakeEngineServer.close(() => resolve()));
2034+
// Make sure we can get the error from reporting.
2035+
const sendingError = await reportErrorPromise;
2036+
expect(sendingError).toBeTruthy();
2037+
if (networkError) {
2038+
expect(sendingError.message).toContain(
2039+
'Error sending report to Apollo Engine servers',
2040+
);
2041+
expect(sendingError.message).toContain('ECONNREFUSED');
2042+
} else {
2043+
expect(sendingError.message).toBe(
2044+
`Error sending report to Apollo Engine servers: HTTP status ${status}, Important text in the body`,
2045+
);
2046+
}
2047+
expect(requestCount).toBe(expectedRequestCount);
2048+
} finally {
2049+
if (!networkError) {
2050+
await closeServer();
2051+
}
2052+
}
20112053
}
2054+
it('with retryable error', async () => {
2055+
await testWithStatus(500, 3);
2056+
});
2057+
it('with network error', async () => {
2058+
await testWithStatus(0, 3);
2059+
});
2060+
it('with non-retryable error', async () => {
2061+
await testWithStatus(400, 1);
2062+
});
20122063
});
20132064
});
20142065

0 commit comments

Comments
 (0)
Please sign in to comment.