Skip to content

Commit

Permalink
Add --detectLeaks and --detectOpenHandles to the unit tests (#4640)
Browse files Browse the repository at this point in the history
* Add --detectLeaks and --detectOpenHandles to the unit tests

* Exclude browser tests for now

* Do not fail fasty

* Do not expose GC or log heap

* More

* Use the same server with other upload example

* chore(dependencies): updated changesets for modified dependencies

* chore(dependencies): updated changesets for modified dependencies

* GO

* chore(dependencies): updated changesets for modified dependencies

* Go

* Again

* chore(dependencies): updated changesets for modified dependencies

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
ardatan and github-actions[bot] committed Aug 12, 2022
1 parent f232ba0 commit 27bdc23
Show file tree
Hide file tree
Showing 31 changed files with 1,175 additions and 1,010 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/apollo-engine-loader": patch
---

dependencies updates:

- Updated dependency [`@whatwg-node/fetch@^0.2.9` ↗︎](https://www.npmjs.com/package/@whatwg-node/fetch/v/^0.2.9) (was `^0.2.4`, in `dependencies`)
7 changes: 7 additions & 0 deletions .changeset/@graphql-tools_batch-delegate-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/batch-delegate": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
7 changes: 7 additions & 0 deletions .changeset/@graphql-tools_github-loader-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/github-loader": patch
---

dependencies updates:

- Updated dependency [`@whatwg-node/fetch@^0.2.9` ↗︎](https://www.npmjs.com/package/@whatwg-node/fetch/v/^0.2.9) (was `^0.2.4`, in `dependencies`)
7 changes: 7 additions & 0 deletions .changeset/@graphql-tools_links-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/links": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
7 changes: 7 additions & 0 deletions .changeset/@graphql-tools_prisma-loader-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/prisma-loader": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/url-loader@7.13.7` ↗︎](https://www.npmjs.com/package/@graphql-tools/url-loader/v/7.13.7) (was `7.13.6`, in `dependencies`)
9 changes: 9 additions & 0 deletions .changeset/@graphql-tools_stitch-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@graphql-tools/stitch": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/batch-delegate@8.3.5` ↗︎](https://www.npmjs.com/package/@graphql-tools/batch-delegate/v/8.3.5) (was `8.3.4`, in `dependencies`)
- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
- Updated dependency [`@graphql-tools/wrap@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/wrap/v/9.0.3) (was `9.0.2`, in `dependencies`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/stitching-directives": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
9 changes: 9 additions & 0 deletions .changeset/@graphql-tools_url-loader-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@graphql-tools/url-loader": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
- Updated dependency [`@graphql-tools/wrap@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/wrap/v/9.0.3) (was `9.0.2`, in `dependencies`)
- Updated dependency [`@whatwg-node/fetch@^0.2.9` ↗︎](https://www.npmjs.com/package/@whatwg-node/fetch/v/^0.2.9) (was `^0.2.4`, in `dependencies`)
7 changes: 7 additions & 0 deletions .changeset/@graphql-tools_wrap-4640-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@graphql-tools/wrap": patch
---

dependencies updates:

- Updated dependency [`@graphql-tools/delegate@9.0.3` ↗︎](https://www.npmjs.com/package/@graphql-tools/delegate/v/9.0.3) (was `9.0.2`, in `dependencies`)
8 changes: 8 additions & 0 deletions .changeset/early-baboons-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@graphql-tools/links': patch
'@graphql-tools/apollo-engine-loader': patch
'@graphql-tools/github-loader': patch
'@graphql-tools/url-loader': patch
---

Bump fetch package
5 changes: 5 additions & 0 deletions .changeset/mighty-eels-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-tools/url-loader': patch
---

Even if 'multipart' is set to true but there is no files in the variables, still use regular JSON request
3 changes: 2 additions & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ jobs:
name: Unit Test on Node ${{matrix.node-version}} (${{matrix.os}}) and GraphQL v${{matrix.graphql_version}}
runs-on: ${{matrix.os}}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest] # remove windows to speed up the tests
node-version: [14, 16, 18]
Expand Down Expand Up @@ -214,6 +215,6 @@ jobs:
- name: Build Packages
run: yarn build
- name: Test
run: yarn test --ci browser
run: yarn jest --no-watchman --ci browser
env:
TEST_BROWSER: true
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"lint": "eslint --ext .ts .",
"prettier": "prettier --ignore-path .prettierignore --write --list-different .",
"prettier:check": "prettier --ignore-path .prettierignore --check .",
"test": "jest --no-watchman --detectOpenHandles --logHeapUsage",
"test": "jest --no-watchman --detectOpenHandles --detectLeaks",
"prerelease": "yarn build",
"release": "changeset publish"
},
Expand Down
1 change: 0 additions & 1 deletion packages/links/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
"devDependencies": {
"@apollo/client": "3.6.9",
"@types/apollo-upload-client": "17.0.1",
"@graphql-yoga/node": "2.13.6",
"graphql-upload": "16.0.1"
},
"dependencies": {
Expand Down
30 changes: 19 additions & 11 deletions packages/links/tests/upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import { AddressInfo } from 'net';
import { Readable } from 'stream';

import express, { Express } from 'express';
import { createServer } from '@graphql-yoga/node';
import GraphQLUpload from 'graphql-upload/GraphQLUpload.mjs';
import graphqlUploadExpress from 'graphql-upload/graphqlUploadExpress.mjs';
import FormData from 'form-data';
import fetch from 'node-fetch';
import { buildSchema } from 'graphql';
import { buildSchema, execute, GraphQLSchema, parse } from 'graphql';

import { makeExecutableSchema } from '@graphql-tools/schema';
import { stitchSchemas } from '@graphql-tools/stitch';
Expand Down Expand Up @@ -57,6 +56,21 @@ function testGraphqlMultipartRequest(query: string, port: number) {
});
}

function getBasicGraphQLMiddleware(schema: GraphQLSchema) {
return (req: any, res: any) => {
Promise.resolve().then(async () => {
const { query, variables, operationName } = req.body;
const result = await execute({
schema,
document: parse(query),
variableValues: variables,
operationName,
});
res.json(result);
});
};
}

describe('graphql upload', () => {
let remoteServer: Server;
let remotePort: number;
Expand Down Expand Up @@ -89,9 +103,8 @@ describe('graphql upload', () => {

const remoteApp = express().use(
graphqlUploadExpress(),
createServer({
schema: remoteSchema,
})
// Yoga causes leak, so we are removing that for now
getBasicGraphQLMiddleware(remoteSchema)
);

remoteServer = await startServer(remoteApp);
Expand Down Expand Up @@ -123,12 +136,7 @@ describe('graphql upload', () => {
},
});

const gatewayApp = express().use(
graphqlUploadExpress(),
createServer({
schema: gatewaySchema,
})
);
const gatewayApp = express().use(graphqlUploadExpress(), getBasicGraphQLMiddleware(gatewaySchema));

gatewayServer = await startServer(gatewayApp);
gatewayPort = (gatewayServer.address() as AddressInfo).port;
Expand Down
2 changes: 1 addition & 1 deletion packages/loaders/apollo-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"dependencies": {
"@ardatan/sync-fetch": "0.0.1",
"@graphql-tools/utils": "8.10.0",
"@whatwg-node/fetch": "^0.2.4",
"@whatwg-node/fetch": "^0.2.9",
"tslib": "^2.4.0"
},
"publishConfig": {
Expand Down
2 changes: 1 addition & 1 deletion packages/loaders/github/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@ardatan/sync-fetch": "0.0.1",
"@graphql-tools/utils": "8.10.0",
"@graphql-tools/graphql-tag-pluck": "7.3.3",
"@whatwg-node/fetch": "^0.2.4",
"@whatwg-node/fetch": "^0.2.9",
"tslib": "^2.4.0"
},
"publishConfig": {
Expand Down
3 changes: 1 addition & 2 deletions packages/loaders/url/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
},
"devDependencies": {
"@envelop/live-query": "4.0.1",
"@graphql-yoga/node": "2.13.6",
"@types/express": "4.17.13",
"@types/extract-files": "8.1.1",
"@types/valid-url": "1.0.3",
Expand All @@ -71,7 +70,7 @@
"@ardatan/sync-fetch": "0.0.1",
"@n1ru4l/graphql-live-query": "^0.10.0",
"@types/ws": "^8.0.0",
"@whatwg-node/fetch": "^0.2.4",
"@whatwg-node/fetch": "^0.2.9",
"dset": "^3.1.2",
"extract-files": "^11.0.0",
"graphql-ws": "^5.4.1",
Expand Down
8 changes: 0 additions & 8 deletions packages/loaders/url/src/addCancelToResponseStream.ts

This file was deleted.

25 changes: 25 additions & 0 deletions packages/loaders/url/src/event-stream/addCancelToResponseStream.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { withCancel } from '@graphql-tools/utils';

export function cancelNeeded() {
if (globalThis.process?.versions?.node) {
const [nodeMajorStr, nodeMinorStr] = process.versions.node.split('.');

const nodeMajor = parseInt(nodeMajorStr);
const nodeMinor = parseInt(nodeMinorStr);

if (nodeMajor > 16 || (nodeMajor === 16 && nodeMinor >= 5)) {
return false;
}
return true;
}

return false;
}

export function addCancelToResponseStream<T>(resultStream: AsyncIterable<T>, controller: AbortController) {
return withCancel(resultStream, () => {
if (!controller.signal.aborted) {
controller.abort();
}
});
}
13 changes: 11 additions & 2 deletions packages/loaders/url/src/event-stream/handleEventStreamResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@ import { ExecutionResult } from 'graphql';
import { inspect, isAsyncIterable } from '@graphql-tools/utils';
import { handleAsyncIterable } from './handleAsyncIterable.js';
import { handleReadableStream } from './handleReadableStream.js';
import { addCancelToResponseStream } from './addCancelToResponseStream.js';

export function isReadableStream(value: any): value is ReadableStream {
return value && typeof value.getReader === 'function';
}

export async function handleEventStreamResponse(response: Response): Promise<AsyncIterable<ExecutionResult>> {
export async function handleEventStreamResponse(
response: Response,
controller?: AbortController
): Promise<AsyncIterable<ExecutionResult>> {
// node-fetch returns body as a promise so we need to resolve it
const body = response.body;
if (body) {
if (isReadableStream(body)) {
return handleReadableStream(body);
}
if (isAsyncIterable<Uint8Array | string>(body)) {
return handleAsyncIterable(body);
const resultStream = handleAsyncIterable(body);
if (controller) {
return addCancelToResponseStream(resultStream, controller);
} else {
return resultStream;
}
}
}
throw new Error('Response body is expected to be a readable stream but got; ' + inspect(body));
Expand Down
12 changes: 10 additions & 2 deletions packages/loaders/url/src/handleMultipartMixedResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { meros as merosIncomingMessage } from 'meros/node';
import { meros as merosReadableStream } from 'meros/browser';
import { mapAsyncIterator } from '@graphql-tools/utils';
import { dset } from 'dset/merge';
import { addCancelToResponseStream } from './event-stream/addCancelToResponseStream.js';

interface ExecutionPatchResult<TData = { [key: string]: any }, TExtensions = { [key: string]: any }> {
errors?: ReadonlyArray<GraphQLError>;
Expand All @@ -28,7 +29,7 @@ function isIncomingMessage(body: any): body is IncomingMessage {
return body != null && typeof body === 'object' && 'pipe' in body;
}

export async function handleMultipartMixedResponse(response: Response) {
export async function handleMultipartMixedResponse(response: Response, controller?: AbortController) {
const body = await (response.body as unknown as Promise<IncomingMessage> | ReadableStream);
const contentType = response.headers.get('content-type') || '';
let asyncIterator: AsyncIterator<Part>;
Expand All @@ -45,7 +46,8 @@ export async function handleMultipartMixedResponse(response: Response) {
}

const executionResult: ExecutionResult = {};
return mapAsyncIterator(asyncIterator, (part: Part) => {

const resultStream = mapAsyncIterator(asyncIterator, (part: Part) => {
if (part.json) {
const chunk = part.body;
if (chunk.path) {
Expand All @@ -67,4 +69,10 @@ export async function handleMultipartMixedResponse(response: Response) {
return executionResult;
}
});

if (controller) {
return addCancelToResponseStream(resultStream, controller);
}

return resultStream;
}

1 comment on commit 27bdc23

@vercel
Copy link

@vercel vercel bot commented on 27bdc23 Aug 12, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.