Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use GraphQL-compliant response payload on error #972

Merged
merged 6 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 16 additions & 36 deletions src/typegate/src/services/artifact_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import { z } from "zod";
import { getLogger } from "../log.ts";
import { BaseError, UnknownError } from "../errors.ts";
import { jsonError } from "./responses.ts";
import { jsonOk } from "./responses.ts";

const logger = getLogger(import.meta);

Expand All @@ -25,9 +27,9 @@
if (operation === "prepare-upload") {
if (request.method !== "POST") {
logger.warn("Method not allowed: {}", request.method);
return new Response(JSON.stringify({ error: "method not allowed" }), {
return jsonError({
message: `method not allowed: ${request.method}`,

Check warning on line 31 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L30-L31

Added lines #L30 - L31 were not covered by tests
status: 405,
headers: { "Content-Type": "application/json" },
});
}

Expand All @@ -36,20 +38,15 @@
metaList = prepareUploadBodySchema.parse(await request.json());
} catch (error) {
logger.error("Failed to parse data: {}", error);
return new Response(
JSON.stringify({ error: `Invalid Request Body: ${error.message}` }),
{
status: 400,
headers: { "Content-Type": "application/json" },
},
);
return jsonError({
message: `invalid request body: ${error.message}`,
status: 400,
});

Check warning on line 44 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L41-L44

Added lines #L41 - L44 were not covered by tests
}

try {
const data = await this.#createUploadTokens(metaList, tgName);
return new Response(JSON.stringify(data), {
headers: { "Content-Type": "application/json" },
});
return jsonOk({ data });
} catch (e) {
if (e instanceof BaseError) {
return e.toResponse();
Expand All @@ -60,28 +57,22 @@

if (operation) {
logger.warn("not found: {} {}", request.method, url.toString());
return new Response(JSON.stringify({ message: "not found" }), {
status: 404,
headers: { "Content-Type": "application/json" },
});
return jsonError({ message: "not found", status: 404 });

Check warning on line 60 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L60

Added line #L60 was not covered by tests
}

if (request.method !== "POST") {
logger.warn("Method not allowed: {}", request.method);
return new Response(JSON.stringify({ error: "method not allowed" }), {
return jsonError({
message: `method not allowed: ${request.method}`,

Check warning on line 66 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L65-L66

Added lines #L65 - L66 were not covered by tests
status: 405,
headers: { "Content-Type": "application/json" },
});
}

const token = url.searchParams.get("token");

if (!token) {
logger.warn("Missing upload token");
return new Response(JSON.stringify({ error: "missing token" }), {
status: 403,
headers: { "Content-Type": "application/json" },
});
return jsonError({ message: "missing token", status: 403 });

Check warning on line 75 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L75

Added line #L75 was not covered by tests
}

return await this.#handleUpload(token, request.body!, tgName);
Expand Down Expand Up @@ -110,10 +101,7 @@
if (e instanceof BaseError) {
return e.toResponse();
}
return new Response(JSON.stringify({ error: e.message }), {
status: 500,
headers: { "Content-Type": "application/json" },
});
return jsonError({ message: e.message, status: 500 });

Check warning on line 104 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L104

Added line #L104 was not covered by tests
}

if (meta.typegraphName !== tgName) {
Expand All @@ -125,17 +113,9 @@
if (hash !== meta.hash) {
await this.store.persistence.delete(hash);
logger.warn("hash mismatch: {} {}", hash, meta.hash);
return new Response(JSON.stringify({ error: "hash mismatch" }), {
status: 403,
headers: { "Content-Type": "application/json" },
});
return jsonError({ message: "hash mismatch", status: 403 });

Check warning on line 116 in src/typegate/src/services/artifact_service.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/artifact_service.ts#L116

Added line #L116 was not covered by tests
}

return new Response(JSON.stringify({ status: "ok" }), {
status: 201,
headers: {
"Content-Type": "application/json",
},
});
return jsonOk({ data: { status: "ok" }, status: 201 });
}
}
6 changes: 2 additions & 4 deletions src/typegate/src/services/auth/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import type { QueryEngine } from "../../engine/query_engine.ts";
import * as routes from "./routes/mod.ts";
import { getLogger } from "../../log.ts";
import { methodNotAllowed } from "../../services/responses.ts";
import { jsonOk, methodNotAllowed } from "../../services/responses.ts";
import type { Runtime } from "../../runtimes/Runtime.ts";

const logger = getLogger(import.meta);
Expand Down Expand Up @@ -132,9 +132,7 @@
uri:
`${url.protocol}//${url.host}/${engine.name}/auth/${name}?redirect_uri=${origin}`,
}));
return new Response(JSON.stringify({ providers }), {
headers: { "content-type": "application/json" },
});
return jsonOk({ data: providers });

Check warning on line 135 in src/typegate/src/services/auth/mod.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/mod.ts#L135

Added line #L135 was not covered by tests
}

return await provider.authMiddleware(request);
Expand Down
9 changes: 6 additions & 3 deletions src/typegate/src/services/auth/protocols/oauth2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
generateWeakValidator,
} from "../../../engine/typecheck/input.ts";
import type { TokenMiddlewareOutput } from "./protocol.ts";
import { jsonError } from "../../responses.ts";

const logger = getLogger(import.meta);

Expand Down Expand Up @@ -205,9 +206,10 @@
new Headers(),
);
// https://github.com/cmd-johnson/deno-oauth2-client/issues/25
return new Response(`invalid oauth2, check your credentials: ${e}`, {
status: 400,
return jsonError({
message: `invalid oauth2, check your credentials: ${e}`,

Check warning on line 210 in src/typegate/src/services/auth/protocols/oauth2.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/protocols/oauth2.ts#L209-L210

Added lines #L209 - L210 were not covered by tests
headers,
status: 400,

Check warning on line 212 in src/typegate/src/services/auth/protocols/oauth2.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/protocols/oauth2.ts#L212

Added line #L212 was not covered by tests
});
}
}
Expand Down Expand Up @@ -237,7 +239,8 @@
});
}

return new Response("missing origin or redirect_uri query parameter", {
return jsonError({
message: "missing origin or redirect_uri query parameter",
status: 400,
});
}
Expand Down
21 changes: 9 additions & 12 deletions src/typegate/src/services/auth/routes/take.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: MPL-2.0

import { getLogger } from "../../../log.ts";
import { jsonError, jsonOk } from "../../responses.ts";
import { clearCookie, getEncryptedCookie } from "../cookies.ts";
import type { RouteParams } from "./mod.ts";

Expand All @@ -20,23 +21,19 @@
);

if (!redirectUri.startsWith(origin)) {
return new Response(
"take request must share domain with redirect uri",
{
status: 400,
headers: resHeaders,
},
);
return jsonError({
status: 400,
message: "take request must share domain with redirect uri",
headers: resHeaders,
});

Check warning on line 28 in src/typegate/src/services/auth/routes/take.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/take.ts#L24-L28

Added lines #L24 - L28 were not covered by tests
}
resHeaders.set("content-type", "application/json");
return new Response(JSON.stringify({ token }), {
status: 200,
headers: resHeaders,
});
return jsonOk({ data: { token }, headers: resHeaders });
} catch (e) {
logger.info(`take request failed ${e}`);
return new Response("not authorized", {
return jsonError({

Check warning on line 34 in src/typegate/src/services/auth/routes/take.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/take.ts#L34

Added line #L34 was not covered by tests
status: 401,
message: "not authorized",

Check warning on line 36 in src/typegate/src/services/auth/routes/take.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/take.ts#L36

Added line #L36 was not covered by tests
headers: resHeaders,
});
}
Expand Down
14 changes: 6 additions & 8 deletions src/typegate/src/services/auth/routes/validate.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// Copyright Metatype OÜ, licensed under the Mozilla Public License Version 2.0.
// SPDX-License-Identifier: MPL-2.0

import { jsonError, jsonOk } from "../../responses.ts";
import type { RouteParams } from "./mod.ts";

export function badRequest(message: string): Response {
return new Response(message, {
status: 400,
headers: { "content-type": "text/plain" },
});
return jsonError({ status: 400, message });

Check warning on line 8 in src/typegate/src/services/auth/routes/validate.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/validate.ts#L8

Added line #L8 was not covered by tests
}

type Action =
Expand Down Expand Up @@ -69,11 +67,11 @@
}

// return json response
return new Response(JSON.stringify(result), {
headers: {
"content-type": "application/json",
return jsonOk({
data: result,
headers: new Headers({

Check warning on line 72 in src/typegate/src/services/auth/routes/validate.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/validate.ts#L70-L72

Added lines #L70 - L72 were not covered by tests
"access-control-allow-origin": "*",
...headers,
},
}),

Check warning on line 75 in src/typegate/src/services/auth/routes/validate.ts

View check run for this annotation

Codecov / codecov/patch

src/typegate/src/services/auth/routes/validate.ts#L75

Added line #L75 was not covered by tests
});
}
14 changes: 7 additions & 7 deletions src/typegate/src/services/graphql_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,30 +119,30 @@ export async function handleGraphQL(
);
}

return jsonOk(res, headers);
return jsonOk({ data: { data: res }, headers });
} catch (e) {
// throw e;
if (e instanceof BaseError) {
return e.toResponse(headers);
}
if (e instanceof ResolverError) {
logger.error(`field err: ${e.message}`);
return jsonError(e.message, headers, 502);
return jsonError({ status: 502, message: e.message, headers });
} else if (e instanceof BadContext) {
logger.error(`context err: ${e.message}`);
return jsonError(
e.message,
return jsonError({
status: Object.keys(context).length === 0 ? 401 : 403,
message: e.message,
headers,
Object.keys(context).length === 0 ? 401 : 403,
);
});
} else {
logger.error(`request err: ${Deno.inspect(e)}`);
if (e.cause) {
logger.error(
Deno.inspect(e.cause, { strAbbreviateSize: 1024, depth: 10 }),
);
}
return jsonError(e.message, headers, 400);
return jsonError({ status: 400, message: e.message, headers });
}
}
}
30 changes: 24 additions & 6 deletions src/typegate/src/services/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,37 @@
import type { JSONValue } from "../utils.ts";
import { BaseError, ErrorKind } from "../errors.ts";

export const jsonOk = (data: JSONValue, headers: Headers) => {
export type JsonOkConfig = {
data: JSONValue;
headers?: Headers | HeadersInit;
status?: number;
};

export const jsonOk = (
{ status = 200, data, headers: headersInit }: JsonOkConfig,
) => {
const headers = headersInit != null
? new Headers(headersInit)
: new Headers();
headers.set("content-type", "application/json");
return new Response(JSON.stringify({ data }), {
status: 200,
return new Response(JSON.stringify(data), {
status,
headers,
});
};

export type JsonErrorConfig = {
status: number;
message: string;
headers?: Headers | HeadersInit;
};

export const jsonError = (
message: string,
headers: Headers,
status: number,
{ status, message, headers: headersInit }: JsonErrorConfig,
) => {
const headers = headersInit != null
? new Headers(headersInit)
: new Headers();
headers.set("content-type", "application/json");
return new Response(
JSON.stringify({
Expand Down
2 changes: 1 addition & 1 deletion src/typegate/src/typegate/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ export class Typegate implements AsyncDisposable {
new Headers(cors),
).catch((e) => e);
if (jwtCheck instanceof Error) {
return jsonError(jwtCheck.message, new Headers(), 401);
return jsonError({ message: jwtCheck.message, status: 401 });
}

const [context, headers] = jwtCheck;
Expand Down
Loading