Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a5e0d9a
repro error improvement
enisdenjo Jan 15, 2025
2ca6847
introduce importer
enisdenjo Jan 15, 2025
8dde1bf
move stuff around
enisdenjo Jan 15, 2025
8832d62
test stuff and export filepath
enisdenjo Jan 15, 2025
dd85cb2
use importer
enisdenjo Jan 15, 2025
580a0d6
path to importer
enisdenjo Jan 15, 2025
b46be27
changeset
enisdenjo Jan 15, 2025
4ccb135
expect fail
enisdenjo Jan 15, 2025
3a664da
changeset
enisdenjo Jan 15, 2025
abf0a96
leftover stack added twice
enisdenjo Jan 15, 2025
4849d17
chore(dependencies): updated changesets for modified dependencies
github-actions[bot] Jan 15, 2025
e342262
exclude synax error files
enisdenjo Jan 16, 2025
8ca68f2
skip formatting files with syntax errors
enisdenjo Jan 16, 2025
cd5022b
bun unit and e2e test and transpile
enisdenjo Jan 16, 2025
953bd9c
no dispose if no exit
enisdenjo Jan 16, 2025
3945fa6
also bun-docker
enisdenjo Jan 16, 2025
59730d6
mount file to docker for e2es
enisdenjo Jan 16, 2025
5bddd48
skip transpile tests when leaktest
enisdenjo Jan 16, 2025
331a8ac
support windows paths
enisdenjo Jan 16, 2025
ce7b131
add container to stack if started
enisdenjo Jan 16, 2025
79c68a0
terminate and kill - no leftover processes
enisdenjo Jan 16, 2025
b321b9b
throw docker errors always
enisdenjo Jan 16, 2025
d10e9b7
debug...
enisdenjo Jan 16, 2025
aafa3f0
debug
enisdenjo Jan 16, 2025
a22fe56
correct message
enisdenjo Jan 16, 2025
8aefe13
Revert "debug"
enisdenjo Jan 16, 2025
9371dc0
Revert "debug..."
enisdenjo Jan 16, 2025
9119b23
retry a few times docker start
enisdenjo Jan 16, 2025
e3e8fac
three docker start retries is enough
enisdenjo Jan 16, 2025
585ab90
break if inspect ok
enisdenjo Jan 17, 2025
74a4400
check string
enisdenjo Jan 17, 2025
81dd34b
debug please tell me where it's throwing
enisdenjo Jan 17, 2025
fa44de3
get stack
enisdenjo Jan 17, 2025
aaeb350
some changes
enisdenjo Jan 17, 2025
e06f4e2
dockererror cause
enisdenjo Jan 17, 2025
cb2ddae
abort ctrl if container died during healtcheck
enisdenjo Jan 17, 2025
4118d9d
try ci now
enisdenjo Jan 17, 2025
b14f00c
terminate if pid
enisdenjo Jan 17, 2025
4b845a2
skip in ci
enisdenjo Jan 17, 2025
244af41
waitforexit ignore whatever exit code
enisdenjo Jan 17, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/@graphql-hive_gateway-462-dependencies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@graphql-hive/gateway': patch
---

dependencies updates:

- Added dependency [`@graphql-hive/importer@workspace:^` ↗︎](https://www.npmjs.com/package/@graphql-hive/importer/v/workspace:^) (to `dependencies`)
- Removed dependency [`@graphql-mesh/include@^0.2.3` ↗︎](https://www.npmjs.com/package/@graphql-mesh/include/v/0.2.3) (from `dependencies`)
5 changes: 5 additions & 0 deletions .changeset/lemon-zebras-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/gateway': patch
---

Use `@graphql-hive/importer` for importing configs and transpiling TypeScript files
5 changes: 5 additions & 0 deletions .changeset/plenty-timers-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/gateway': minor
---

Point to exact location of syntax error when parsing malformed config files
5 changes: 5 additions & 0 deletions .changeset/strong-ghosts-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-hive/importer': major
---

Improving Hive's importing capabilities allowing it to parse TypeScript files
3 changes: 3 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ __generated__
.wrangler/
*.Dockerfile
/examples/
/packages/importer/tests/fixtures/syntax-error.ts
/e2e/config-syntax-error/gateway.config.ts
/e2e/config-syntax-error/custom-resolvers.ts
38 changes: 38 additions & 0 deletions e2e/config-syntax-error/config-syntax-error.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { createTenv } from '@internal/e2e';
import { isCI } from '@internal/testing';
import { expect, it } from 'vitest';

const { gateway, service, gatewayRunner } = createTenv(__dirname);

it.skipIf(
// for whatever reason docker in CI sometimes (sometimes is the keyword, more than less)
// doesnt provide all the logs and throws errors with weird messages and I dont know where from or why
// see https://github.com/graphql-hive/gateway/actions/runs/12830196184/job/35777821364
isCI() && gatewayRunner === 'docker',
)(
'should point to exact location of syntax error when parsing a malformed config',
async () => {
await expect(
gateway({
supergraph: {
with: 'mesh',
services: [await service('hello')],
},
runner: {
docker: {
volumes: [
{
host: 'custom-resolvers.ts',
container: '/gateway/custom-resolvers.ts',
},
],
},
},
}),
).rejects.toThrowError(
gatewayRunner === 'bun' || gatewayRunner === 'bun-docker'
? /error: Expected "{" but found "hello"(.|\n)*\/custom-resolvers.ts:8:11/
: /SyntaxError \[Error\]: Error transforming .*(\/|\\)custom-resolvers.ts: Unexpected token, expected "{" \(8:11\)/,
);
},
);
12 changes: 12 additions & 0 deletions e2e/config-syntax-error/custom-resolvers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// @ts-nocheck -- syntax error intentionally

import { GatewayContext } from '@graphql-hive/gateway';
import { IResolvers } from '@graphql-tools/utils';

export const customResolvers: IResolvers<unknown, GatewayContext> = {
Query: {
bye() hello {
return 'world';
},
},
};
6 changes: 6 additions & 0 deletions e2e/config-syntax-error/gateway.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { defineConfig } from '@graphql-hive/gateway';
import { customResolvers } from './custom-resolvers';

export const gatewayConfig = defineConfig({
additionalResolvers: [customResolvers],
});
22 changes: 22 additions & 0 deletions e2e/config-syntax-error/mesh.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {
defineConfig,
loadGraphQLHTTPSubgraph,
} from '@graphql-mesh/compose-cli';
import { Opts } from '@internal/testing';

const opts = Opts(process.argv);

export const composeConfig = defineConfig({
subgraphs: [
{
sourceHandler: loadGraphQLHTTPSubgraph('hello', {
endpoint: `http://localhost:${opts.getServicePort('hello')}/graphql`,
}),
},
],
additionalTypeDefs: /* GraphQL */ `
extend type Query {
bye: String!
}
`,
});
10 changes: 10 additions & 0 deletions e2e/config-syntax-error/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@e2e/config-syntax-error",
"private": true,
"dependencies": {
"@graphql-mesh/compose-cli": "^1.2.13",
"@graphql-tools/utils": "^10.7.2",
"graphql": "^16.9.0",
"tslib": "^2.8.1"
}
}
23 changes: 23 additions & 0 deletions e2e/config-syntax-error/services/hello.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { createServer } from 'http';
import { Opts } from '@internal/testing';
import { createSchema, createYoga } from 'graphql-yoga';

const opts = Opts(process.argv);

createServer(
createYoga({
maskedErrors: false,
schema: createSchema<any>({
typeDefs: /* GraphQL */ `
type Query {
hello: String!
}
`,
resolvers: {
Query: {
hello: () => 'world',
},
},
}),
}),
).listen(opts.getServicePort('hello'));
2 changes: 1 addition & 1 deletion e2e/tsconfig-paths/tsconfig-paths.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ it.skipIf(gatewayRunner.includes('bun'))('should start gateway', async () => {
'type Query { hello: String }',
),
env: {
MESH_INCLUDE_TSCONFIG_SEARCH_PATH: 'tsconfig-paths.tsconfig.json',
HIVE_IMPORTER_TSCONFIG_SEARCH_PATH: 'tsconfig-paths.tsconfig.json',
},
runner: {
docker: {
Expand Down
46 changes: 30 additions & 16 deletions internal/e2e/src/tenv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,6 @@ export function createTenv(cwd: string): Tenv {
path.resolve(__project, 'packages', 'gateway', 'src', 'bin.ts'),
...getFullArgs(),
);
leftoverStack.use(proc);
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed because the same process was added twice to the stack.

break;
}
case 'bin': {
Expand Down Expand Up @@ -883,28 +882,38 @@ export function createTenv(cwd: string): Tenv {
getStats() {
throw new Error('Cannot get stats of a container.');
},
[DisposableSymbols.asyncDispose]() {
async [DisposableSymbols.asyncDispose]() {
if (ctrl.signal.aborted) {
// noop if already disposed
return undefined as unknown as Promise<void>;
return;
}
ctrl.abort();
return ctr.stop({ t: 0, signal: 'SIGTERM' });
await ctr.stop({ t: 0, signal: 'SIGTERM' });
},
};
leftoverStack.use(container);

// verify that the container has started
await setTimeout(interval);
try {
await ctr.inspect();
} catch (err) {
if (Object(err).statusCode === 404) {
throw new DockerError('Container was not started', container);
let startCheckRetries = 3;
while (startCheckRetries) {
await setTimeout(interval);
try {
await ctr.inspect({ abortSignal: ctrl.signal });
break;
} catch (err) {
// we dont use the err.statusCode because it doesnt work in CI, why? no clue
if (/no such container/i.test(String(err))) {
if (!--startCheckRetries) {
throw new DockerError('Container did not start', container, err);
}
continue;
}
throw new DockerError(String(err), container, err);
}
throw err;
}

// we add the container to the stack only if it started
leftoverStack.use(container);

// wait for healthy
if (healthcheck.length > 0) {
while (!ctrl.signal.aborted) {
Expand All @@ -915,21 +924,23 @@ export function createTenv(cwd: string): Tenv {
} = await ctr.inspect({ abortSignal: ctrl.signal });
status = Health?.Status ? String(Health?.Status) : '';
} catch (err) {
if (Object(err).statusCode === 404) {
throw new DockerError('Container was not started', container);
if (/no such container/i.test(String(err))) {
ctrl.abort(); // container died so no need to dispose of it (see async dispose implementation)
throw new DockerError('Container died', container, err);
}
throw err;
throw new DockerError(String(err), container, err);
}

if (status === 'none') {
await container[DisposableSymbols.asyncDispose]();
throw new DockerError(
'Container has "none" health status, but has a healthcheck',
container,
null,
);
} else if (status === 'unhealthy') {
await container[DisposableSymbols.asyncDispose]();
throw new DockerError('Container is unhealthy', container);
throw new DockerError('Container is unhealthy', container, null);
} else if (status === 'healthy') {
break;
} else if (status === 'starting') {
Expand All @@ -938,6 +949,7 @@ export function createTenv(cwd: string): Tenv {
throw new DockerError(
`Unknown health status "${status}"`,
container,
null,
);
}
}
Expand Down Expand Up @@ -1010,10 +1022,12 @@ class DockerError extends Error {
constructor(
public override message: string,
container: Container,
cause: unknown,
) {
super();
this.name = 'DockerError';
this.message = message + '\n' + container.getStd('both');
this.cause = cause;
}
}

Expand Down
17 changes: 12 additions & 5 deletions internal/proc/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,19 @@
mem: parseFloat(mem!) * 0.001, // KB to MB
};
},
[DisposableSymbols.asyncDispose]: () => {
const childPid = child.pid;
if (childPid && !exited) {
return terminate(childPid);
[DisposableSymbols.asyncDispose]: async () => {
if (exited) {
// there's nothing to dispose since the process already exitted (error or not)
return Promise.resolve();
}
return waitForExit;
if (child.pid) {
await terminate(child.pid);
}
child.kill();
await waitForExit.catch(() => {
// we dont care about if abnormal exit code when disposing
// specifically in Windows, exit code is always 1 when killing a live process
});
},
};
stack?.use(proc);
Expand Down Expand Up @@ -140,7 +147,7 @@
// process ended _and_ the stdio streams have been closed
if (code) {
exitDeferred.reject(
new Error(

Check failure on line 150 in internal/proc/src/index.ts

View workflow job for this annotation

GitHub Actions / E2E / Node Binary on Windows

e2e/federation-batching-plan/federation-batching-plan.e2e.ts > should consistently explain the query plan

Error: Exit code 1 from D:\a\gateway\gateway\packages\gateway\hive-gateway --port=50472 supergraph C:\Users\RUNNER~1\AppData\Local\Temp\hive-gateway_e2e_fszRtIuJ\supergraph.graphql node:internal/modules/cjs/loader:1247 throw err; ^ Error: Cannot find module './version.js' Require stack: - C:\Users\RUNNER~1\AppData\Local\Temp\mesh-serve_084d99ccdf8e8467419350956cc91804aaf16431625403a07c6e482c6acdbad1_node_modules\graphql\index.js - packages\gateway\hive-gateway.exe at node:internal/modules/cjs/loader:1244:15 at Module._resolveFilename (bundle/hive-gateway.cjs:3419:14) at Function._load (node:internal/modules/cjs/loader:1070:27) at TracingChannel.traceSync (node:diagnostics_channel:322:14) at wrapModuleLoad (node:internal/modules/cjs/loader:217:24) at Module.require (node:internal/modules/cjs/loader:1335:12) at require (node:internal/modules/helpers:136:16) at Object.<anonymous> (C:\Users\RUNNER~1\AppData\Local\Temp\mesh-serve_084d99ccdf8e8467419350956cc91804aaf16431625403a07c6e482c6acdbad1_node_modules\graphql\index.js:1273:16) at Module._compile (node:internal/modules/cjs/loader:1562:14) at Object..js (node:internal/modules/cjs/loader:1699:10) { code: 'MODULE_NOT_FOUND', requireStack: [ 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\mesh-serve_084d99ccdf8e8467419350956cc91804aaf16431625403a07c6e482c6acdbad1_node_modules\\graphql\\index.js', 'D:\\a\\gateway\\gateway\\packages\\gateway\\hive-gateway.exe' ] } Node.js v22.13.0 ❯ Module._resolveFilename bundle/hive-gateway.cjs:3419:14 ❯ Object.<anonymous> C:/Users/RUNNER~1/AppData/Local/Temp/mesh-serve_084d99ccdf8e8467419350956cc91804aaf16431625403a07c6e482c6acdbad1_node_modules/graphql/index.js:1273:16 ❯ ChildProcess.<anonymous> internal/proc/src/index.ts:150:9
`Exit code ${code} from ${cmd} ${args.join(' ')}\n${trimError(stdboth)}`,
),
);
Expand Down
2 changes: 1 addition & 1 deletion packages/gateway/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@
"@commander-js/extra-typings": "^13.0.0",
"@envelop/core": "^5.0.2",
"@graphql-hive/gateway-runtime": "workspace:^",
"@graphql-hive/importer": "workspace:^",
"@graphql-mesh/cache-cfw-kv": "^0.104.0",
"@graphql-mesh/cache-localforage": "^0.103.0",
"@graphql-mesh/cache-redis": "^0.103.0",
"@graphql-mesh/cross-helpers": "^0.4.9",
"@graphql-mesh/hmac-upstream-signature": "workspace:^",
"@graphql-mesh/include": "^0.2.3",
"@graphql-mesh/plugin-deduplicate-request": "^0.103.0",
"@graphql-mesh/plugin-http-cache": "^0.103.0",
"@graphql-mesh/plugin-jit": "^0.1.0",
Expand Down
8 changes: 4 additions & 4 deletions packages/gateway/rollup.config.binary.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ return module.exports;
JSON.stringify(__MODULES_HASH__),
);

// replace all "graphql*" requires to use the packed deps (the new require will invoke @graphql-mesh/include/hooks)
// replace all "graphql*" requires to use the packed deps (the new require will invoke @graphql-hive/importer/hooks)
for (const [match, path] of code.matchAll(/require\('(graphql.*)'\)/g)) {
code = code.replace(
match,
Expand All @@ -101,13 +101,13 @@ return module.exports;
);
}

// replace the @graphql-mesh/include/hooks register to use the absolute path of the packed deps
// replace the @graphql-hive/importer/hooks register to use the absolute path of the packed deps
const includeHooksRegisterDest =
/register\(\s*'@graphql-mesh\/include\/hooks'/g; // intentionally no closing bracked because there's more arguments
/register\(\s*'@graphql-hive\/importer\/hooks'/g; // intentionally no closing bracked because there's more arguments
if (includeHooksRegisterDest.test(code)) {
code = code.replaceAll(
includeHooksRegisterDest,
`register(require('node:url').pathToFileURL(require('node:path').join(globalThis.__PACKED_DEPS_PATH__, '@graphql-mesh', 'include', 'hooks.mjs'))`,
`register(require('node:url').pathToFileURL(require('node:path').join(globalThis.__PACKED_DEPS_PATH__, '@graphql-hive', 'importer', 'hooks.mjs'))`,
);
} else {
throw new Error(
Expand Down
5 changes: 2 additions & 3 deletions packages/gateway/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ const deps = {
'node_modules/@graphql-hive/gateway-runtime/index': '../runtime/src/index.ts',
'node_modules/@graphql-mesh/fusion-runtime/index':
'../fusion-runtime/src/index.ts',
'node_modules/@graphql-mesh/include/hooks':
'../../node_modules/@graphql-mesh/include/esm/hooks.js',
'node_modules/@graphql-hive/importer/hooks': '../importer/src/hooks.ts',

// default transports should be in the container
'node_modules/@graphql-mesh/transport-common/index':
Expand Down Expand Up @@ -141,7 +140,7 @@ function packagejson() {
const mjsFile = path
.basename(bundle.fileName, '.mjs')
.replace(/\\/g, '/');
// if the bundled file is not "index", then it's an exports path (like with @graphql-mesh/include/hooks)
// if the bundled file is not "index", then it's an package.json exports path
pkg['exports'] = { [`./${mjsFile}`]: `./${bundledFile}` };
}
this.emitFile({
Expand Down
4 changes: 2 additions & 2 deletions packages/gateway/src/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
import 'dotenv/config'; // inject dotenv options to process.env

import module from 'node:module';
import type { InitializeData } from '@graphql-mesh/include/hooks';
import type { InitializeData } from '@graphql-hive/importer/hooks';
import { DefaultLogger } from '@graphql-mesh/utils';
import { enableModuleCachingIfPossible, handleNodeWarnings, run } from './cli';

// @inject-version globalThis.__VERSION__ here

module.register('@graphql-mesh/include/hooks', {
module.register('@graphql-hive/importer/hooks', {
parentURL:
// @ts-ignore bob will complain when bundling for cjs
import.meta.url,
Expand Down
7 changes: 7 additions & 0 deletions packages/importer/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# @graphql-hive/importer

Used to improve Hive's importing capabilities allowing it to parse TypeScript files.

Please note that `get-tsconfig` and `sucrase` are **intentionally** inside devDependencies at the [package.json](/packages/mporter/package.json) because we want to bundle them in.

[pkgroll will bundle all devDependencies that are used in the source code.](https://github.com/privatenumber/pkgroll?tab=readme-ov-file#dependency-bundling--externalization)
Loading
Loading