diff --git a/packages/apollo-engine-reporting/package.json b/packages/apollo-engine-reporting/package.json index e224ca6ce90..4f9257ea7cb 100644 --- a/packages/apollo-engine-reporting/package.json +++ b/packages/apollo-engine-reporting/package.json @@ -21,23 +21,21 @@ "node": ">=6.0" }, "dependencies": { - "apollo-server-env": "^2.0.0-rc.3", "apollo-engine-reporting-protobuf": "0.0.0-beta.7", + "apollo-server-env": "^2.0.0-rc.5", + "async-retry": "^1.2.1", "graphql-extensions": "^0.1.0-beta.15", - "lodash": "^4.17.10", - "requestretry": "^1.13.0" + "lodash": "^4.17.10" }, "devDependencies": { + "@types/async-retry": "^1.2.1", "@types/graphql": "^0.13.1", "@types/jest": "^23.1.2", "@types/lodash": "^4.14.110", - "@types/node-fetch": "^2.1.1", - "@types/requestretry": "^1.12.3", "graphql": "^0.13.2", "graphql-tag": "^2.9.2", "graphql-tools": "^3.0.4", "jest": "^23.2.0", - "node-fetch": "^2.1.2", "ts-jest": "^22.4.6", "tslint": "^5.10.0" }, diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index e6e58a64342..580d36d6a19 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -1,6 +1,5 @@ import * as os from 'os'; import { gzip } from 'zlib'; -import * as request from 'requestretry'; import { DocumentNode } from 'graphql'; import { FullTracesReport, @@ -9,6 +8,9 @@ import { Trace, } from 'apollo-engine-reporting-protobuf'; +import { fetch, Response } from 'apollo-server-env'; +import * as retry from 'async-retry'; + import { EngineReportingExtension } from './extension'; // Override the generated protobuf Traces.encode function so that it will look @@ -170,58 +172,57 @@ export class EngineReportingAgent { } } - public sendReport(): Promise { + public async sendReport(): Promise { const report = this.report; this.resetReport(); if (Object.keys(report.tracesPerQuery).length === 0) { - return Promise.resolve(); + return; } // Send traces asynchronously, so that (eg) addTrace inside a resolver // doesn't block on it. - return Promise.resolve() - .then(() => { - if (this.options.debugPrintReports) { - // tslint:disable-next-line no-console - console.log( - `Engine sending report: ${JSON.stringify(report.toJSON())}`, - ); - } + await Promise.resolve(); + + if (this.options.debugPrintReports) { + // tslint:disable-next-line no-console + console.log(`Engine sending report: ${JSON.stringify(report.toJSON())}`); + } + + const protobufError = FullTracesReport.verify(report); + if (protobufError) { + throw new Error(`Error encoding report: ${protobufError}`); + } + const message = FullTracesReport.encode(report).finish(); - const protobufError = FullTracesReport.verify(report); - if (protobufError) { - throw new Error(`Error encoding report: ${protobufError}`); + const compressed = await new Promise((resolve, reject) => { + // The protobuf library gives us a Uint8Array. Node 8's zlib lets us + // pass it directly; convert for the sake of Node 6. (No support right + // now for Node 4, which lacks Buffer.from.) + const messageBuffer = Buffer.from( + message.buffer as ArrayBuffer, + message.byteOffset, + message.byteLength, + ); + gzip(messageBuffer, (err, compressed) => { + if (err) { + reject(err); + } else { + resolve(compressed); } - const message = FullTracesReport.encode(report).finish(); + }); + }); - return new Promise((resolve, reject) => { - // The protobuf library gives us a Uint8Array. Node 8's zlib lets us - // pass it directly; convert for the sake of Node 6. (No support right - // now for Node 4, which lacks Buffer.from.) - const messageBuffer = Buffer.from( - message.buffer as ArrayBuffer, - message.byteOffset, - message.byteLength, - ); - gzip(messageBuffer, (err, compressed) => { - if (err) { - reject(err); - } else { - resolve(compressed); - } - }); - }); - }) - .then(compressed => { - // Grab this here because the delayStrategy function has a different 'this'. - const minimumRetryDelayMs = this.options.minimumRetryDelayMs || 100; + const endpointUrl = + (this.options.endpointUrl || 'https://engine-report.apollodata.com') + + '/api/ingress/traces'; - // note: retryrequest has built-in Promise support, unlike the base 'request'. - return (request({ - url: - (this.options.endpointUrl || - 'https://engine-report.apollodata.com') + '/api/ingress/traces', + // Wrap fetch with async-retry for automatic retrying + const response: Response = await retry( + // Retry on network errors and 5xx HTTP + // responses. + async () => { + const response = await fetch(endpointUrl, { method: 'POST', headers: { 'user-agent': 'apollo-engine-reporting', @@ -229,42 +230,36 @@ export class EngineReportingAgent { 'content-encoding': 'gzip', }, body: compressed, - // By default, retryrequest will retry on network errors and 5xx HTTP - // responses. - maxAttempts: this.options.maxAttempts || 5, - // Note: use a non-arrow function as this API gives us useful information - // on 'this', and use an 'as any' because the type definitions don't know - // about the function version of this parameter. - delayStrategy: function() { - return Math.pow(minimumRetryDelayMs * 2, this.attempts); - }, - // XXX Back in Optics, we had an explicit proxyUrl option for corporate - // proxies. I was never clear on why `request`'s handling of the - // standard env vars wasn't good enough (see - // https://github.com/apollographql/optics-agent-js/pull/70#discussion_r89374066). - // We may have to add it here. - - // Include 'as any's because @types/requestretry doesn't understand the - // promise API or delayStrategy. - } as any) as any).catch((err: Error) => { - throw new Error(`Error sending report to Engine servers: ${err}`); }); - }) - .then(response => { - if (response.statusCode < 200 || response.statusCode >= 300) { - // Note that we don't expect to see a 3xx here because request follows - // redirects. - throw new Error( - `Error sending report to Engine servers (HTTP status ${ - response.statusCode - }): ${response.body}`, - ); - } - if (this.options.debugPrintReports) { - // tslint:disable-next-line no-console - console.log(`Engine report: status ${response.statusCode}`); + + if (response.status >= 500 && response.status < 600) { + throw new Error(`${response.status}: ${response.statusText}`); + } else { + return response; } - }); + }, + { + retries: this.options.maxAttempts || 5, + minTimeout: this.options.minimumRetryDelayMs || 100, + factor: 2, + }, + ).catch((err: Error) => { + throw new Error(`Error sending report to Engine servers: ${err}`); + }); + + if (response.status < 200 || response.status >= 300) { + // Note that we don't expect to see a 3xx here because request follows + // redirects. + throw new Error( + `Error sending report to Engine servers (HTTP status ${ + response.status + }): ${await response.text()}`, + ); + } + if (this.options.debugPrintReports) { + // tslint:disable-next-line no-console + console.log(`Engine report: status ${response.status}`); + } } // Stop prevents reports from being sent automatically due to time or buffer diff --git a/packages/apollo-server-env/src/fetch.d.ts b/packages/apollo-server-env/src/fetch.d.ts index 2dd19133d26..d40edd9b521 100644 --- a/packages/apollo-server-env/src/fetch.d.ts +++ b/packages/apollo-server-env/src/fetch.d.ts @@ -98,4 +98,4 @@ export interface ResponseInit { statusText?: string; } -export type BodyInit = ArrayBuffer | string; +export type BodyInit = ArrayBuffer | ArrayBufferView | string; diff --git a/packages/apollo-server-env/tsconfig.json b/packages/apollo-server-env/tsconfig.json index 8f8782339c9..a7cc54c645b 100644 --- a/packages/apollo-server-env/tsconfig.json +++ b/packages/apollo-server-env/tsconfig.json @@ -15,5 +15,5 @@ }, "include": ["src/**/*"], "exclude": ["node_modules", "**/__tests__/*", "**/__mocks__/*"], - "types": ["node"] + "types": [] } diff --git a/packages/apollo-server-integration-testsuite/package.json b/packages/apollo-server-integration-testsuite/package.json index 9ea6df58555..3147d72b5e3 100644 --- a/packages/apollo-server-integration-testsuite/package.json +++ b/packages/apollo-server-integration-testsuite/package.json @@ -32,7 +32,7 @@ "apollo-link": "^1.2.2", "apollo-link-http": "^1.5.4", "apollo-link-persisted-queries": "^0.2.1", - "apollo-server-env": "^2.0.0-rc.3", + "apollo-server-env": "^2.0.0-rc.5", "body-parser": "^1.18.3", "graphql-extensions": "^0.1.0-beta.15", "graphql-subscriptions": "^0.5.8", diff --git a/packages/graphql-extensions/package.json b/packages/graphql-extensions/package.json index d32931772b7..35ccac3f353 100644 --- a/packages/graphql-extensions/package.json +++ b/packages/graphql-extensions/package.json @@ -22,7 +22,7 @@ "node": ">=6.0" }, "dependencies": { - "apollo-server-env": "^2.0.0-rc.3", + "apollo-server-env": "^2.0.0-rc.5", "core-js": "^2.5.7", "source-map-support": "^0.5.6" },