From 4d5a4da4e59c1b965784e14e7e2a7d0243ef76d5 Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 29 Jun 2018 09:04:53 +0200 Subject: [PATCH 1/7] Remove Node dependencies from package.json --- packages/apollo-engine-reporting/package.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/apollo-engine-reporting/package.json b/packages/apollo-engine-reporting/package.json index e224ca6ce90..9933b9972ba 100644 --- a/packages/apollo-engine-reporting/package.json +++ b/packages/apollo-engine-reporting/package.json @@ -24,20 +24,16 @@ "apollo-server-env": "^2.0.0-rc.3", "apollo-engine-reporting-protobuf": "0.0.0-beta.7", "graphql-extensions": "^0.1.0-beta.15", - "lodash": "^4.17.10", - "requestretry": "^1.13.0" + "lodash": "^4.17.10" }, "devDependencies": { "@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" }, From 15ebaa84d0192b286858c46950349b833866154f Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 29 Jun 2018 11:55:59 +0200 Subject: [PATCH 2/7] Replace Node request with fetch --- packages/apollo-engine-reporting/src/agent.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index e6e58a64342..2124b5e496c 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, @@ -8,6 +7,7 @@ import { Traces, Trace, } from 'apollo-engine-reporting-protobuf'; +import { fetch } from 'apollo-server-env'; import { EngineReportingExtension } from './extension'; @@ -218,10 +218,10 @@ export class EngineReportingAgent { const minimumRetryDelayMs = this.options.minimumRetryDelayMs || 100; // 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', + const endpointUrl = + this.options.endpointUrl || + 'https://engine-report.apollodata.com' + '/api/ingress/traces'; + return (fetch(endpointUrl, { method: 'POST', headers: { 'user-agent': 'apollo-engine-reporting', From 7a9dbafb747df54a19841231d1e0bff177e86a13 Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 29 Jun 2018 15:23:12 +0200 Subject: [PATCH 3/7] Wrap fetch in @zeit/fetch-retry, convert to async/await, and fix types --- packages/apollo-engine-reporting/package.json | 3 +- packages/apollo-engine-reporting/src/agent.ts | 161 +++++++++--------- packages/apollo-server-env/src/fetch.d.ts | 2 +- packages/apollo-server-env/tsconfig.json | 2 +- 4 files changed, 82 insertions(+), 86 deletions(-) diff --git a/packages/apollo-engine-reporting/package.json b/packages/apollo-engine-reporting/package.json index 9933b9972ba..b33d8f80d7e 100644 --- a/packages/apollo-engine-reporting/package.json +++ b/packages/apollo-engine-reporting/package.json @@ -21,8 +21,9 @@ "node": ">=6.0" }, "dependencies": { - "apollo-server-env": "^2.0.0-rc.3", + "@zeit/fetch-retry": "^3.0.0", "apollo-engine-reporting-protobuf": "0.0.0-beta.7", + "apollo-server-env": "^2.0.0-rc.3", "graphql-extensions": "^0.1.0-beta.15", "lodash": "^4.17.10" }, diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 2124b5e496c..68017fb6c96 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -7,7 +7,18 @@ import { Traces, Trace, } from 'apollo-engine-reporting-protobuf'; -import { fetch } from 'apollo-server-env'; + +import { + fetch as originalFetch, + RequestInfo, + RequestInit, + Response, +} from 'apollo-server-env'; +// Wrap fetch with @zeit/fetch-retry for automatic retrying +const fetch = require('@zeit/fetch-retry')(originalFetch) as ( + input?: RequestInfo, + init?: RequestInit & { retries: number; minTimeout: number }, +) => Promise; import { EngineReportingExtension } from './extension'; @@ -170,101 +181,85 @@ 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())}`, - ); - } - - const protobufError = FullTracesReport.verify(report); - if (protobufError) { - throw new Error(`Error encoding report: ${protobufError}`); - } - const message = FullTracesReport.encode(report).finish(); + await Promise.resolve(); - 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; + if (this.options.debugPrintReports) { + // tslint:disable-next-line no-console + console.log(`Engine sending report: ${JSON.stringify(report.toJSON())}`); + } - // note: retryrequest has built-in Promise support, unlike the base 'request'. - const endpointUrl = - this.options.endpointUrl || - 'https://engine-report.apollodata.com' + '/api/ingress/traces'; - return (fetch(endpointUrl, { - method: 'POST', - headers: { - 'user-agent': 'apollo-engine-reporting', - 'x-api-key': this.apiKey, - '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. + const protobufError = FullTracesReport.verify(report); + if (protobufError) { + throw new Error(`Error encoding report: ${protobufError}`); + } + const message = FullTracesReport.encode(report).finish(); - // 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}`); + 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 endpointUrl = + (this.options.endpointUrl || 'https://engine-report.apollodata.com') + + '/api/ingress/traces'; + + const response = await fetch(endpointUrl, { + method: 'POST', + headers: { + 'user-agent': 'apollo-engine-reporting', + 'x-api-key': this.apiKey, + 'content-encoding': 'gzip', + }, + body: compressed, + // By default, fetch-retry will retry on network errors and 5xx HTTP + // responses. + retries: this.options.maxAttempts || 5, + minTimeout: this.options.minimumRetryDelayMs || 100, + // 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. + }).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": [] } From f72e4e8072a83f80b5da62181ce738f888de9528 Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Fri, 29 Jun 2018 16:44:33 +0200 Subject: [PATCH 4/7] Use async-retry directly because @zeit/fetch-retry doesn't support Node 6 --- packages/apollo-engine-reporting/package.json | 3 +- packages/apollo-engine-reporting/src/agent.ts | 55 +++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/packages/apollo-engine-reporting/package.json b/packages/apollo-engine-reporting/package.json index b33d8f80d7e..b5dbf3a56bb 100644 --- a/packages/apollo-engine-reporting/package.json +++ b/packages/apollo-engine-reporting/package.json @@ -21,13 +21,14 @@ "node": ">=6.0" }, "dependencies": { - "@zeit/fetch-retry": "^3.0.0", "apollo-engine-reporting-protobuf": "0.0.0-beta.7", "apollo-server-env": "^2.0.0-rc.3", + "async-retry": "^1.2.1", "graphql-extensions": "^0.1.0-beta.15", "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", diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 68017fb6c96..ff23647fffd 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -8,17 +8,8 @@ import { Trace, } from 'apollo-engine-reporting-protobuf'; -import { - fetch as originalFetch, - RequestInfo, - RequestInit, - Response, -} from 'apollo-server-env'; -// Wrap fetch with @zeit/fetch-retry for automatic retrying -const fetch = require('@zeit/fetch-retry')(originalFetch) as ( - input?: RequestInfo, - init?: RequestInit & { retries: number; minTimeout: number }, -) => Promise; +import { fetch, Response } from 'apollo-server-env'; +import * as retry from 'async-retry'; import { EngineReportingExtension } from './extension'; @@ -226,24 +217,32 @@ export class EngineReportingAgent { (this.options.endpointUrl || 'https://engine-report.apollodata.com') + '/api/ingress/traces'; - const response = await fetch(endpointUrl, { - method: 'POST', - headers: { - 'user-agent': 'apollo-engine-reporting', - 'x-api-key': this.apiKey, - 'content-encoding': 'gzip', - }, - body: compressed, - // By default, fetch-retry will retry on network errors and 5xx HTTP + // Wrap fetch with async-retry for automatic retrying + const response: Response = await retry( + // Retry on network errors and 5xx HTTP // responses. - retries: this.options.maxAttempts || 5, - minTimeout: this.options.minimumRetryDelayMs || 100, - // 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. - }).catch((err: Error) => { + async () => { + const response = await fetch(endpointUrl, { + method: 'POST', + headers: { + 'user-agent': 'apollo-engine-reporting', + 'x-api-key': this.apiKey, + 'content-encoding': 'gzip', + }, + body: compressed, + }); + + 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, + }, + ).catch((err: Error) => { throw new Error(`Error sending report to Engine servers: ${err}`); }); From aa87e974e5e59920b8c6c4e056456028f7a6da5e Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 29 Jun 2018 10:59:35 -0700 Subject: [PATCH 5/7] bump server-env version in graphql-extensions --- packages/graphql-extensions/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" }, From adec27af120d10e3ca056deee6b29e66252b8779 Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 29 Jun 2018 11:00:16 -0700 Subject: [PATCH 6/7] explicitly include factor in async-retry --- packages/apollo-engine-reporting/src/agent.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index ff23647fffd..580d36d6a19 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -241,6 +241,7 @@ export class EngineReportingAgent { { 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}`); From b7f576ad00e8edab2de4e8a80311537e366d34be Mon Sep 17 00:00:00 2001 From: Evans Hauser Date: Fri, 29 Jun 2018 11:05:55 -0700 Subject: [PATCH 7/7] change apollo-server-env to rc.5 --- packages/apollo-engine-reporting/package.json | 2 +- packages/apollo-server-integration-testsuite/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/apollo-engine-reporting/package.json b/packages/apollo-engine-reporting/package.json index b5dbf3a56bb..4f9257ea7cb 100644 --- a/packages/apollo-engine-reporting/package.json +++ b/packages/apollo-engine-reporting/package.json @@ -22,7 +22,7 @@ }, "dependencies": { "apollo-engine-reporting-protobuf": "0.0.0-beta.7", - "apollo-server-env": "^2.0.0-rc.3", + "apollo-server-env": "^2.0.0-rc.5", "async-retry": "^1.2.1", "graphql-extensions": "^0.1.0-beta.15", "lodash": "^4.17.10" 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",