Skip to content

Commit

Permalink
@graphql-hive/importer and point to exact syntax error while parsing …
Browse files Browse the repository at this point in the history
…config (#462)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
enisdenjo and github-actions[bot] authored Jan 17, 2025
1 parent 5b69bdd commit 9a6ae85
Show file tree
Hide file tree
Showing 30 changed files with 587 additions and 34 deletions.
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);
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 @@ export function spawn(
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
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

0 comments on commit 9a6ae85

Please sign in to comment.