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 4 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 {
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 @@ export class ArtifactService {
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}`,
status: 405,
headers: { "Content-Type": "application/json" },
});
}

Expand All @@ -36,20 +38,15 @@ export class ArtifactService {
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,
});
}

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

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 });
}

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}`,
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 });
}

return await this.#handleUpload(token, request.body!, tgName);
Expand Down Expand Up @@ -110,10 +101,7 @@ export class ArtifactService {
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 });
}

if (meta.typegraphName !== tgName) {
Expand All @@ -125,17 +113,9 @@ export class ArtifactService {
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 });
}

return new Response(JSON.stringify({ status: "ok" }), {
status: 201,
headers: {
"Content-Type": "application/json",
},
});
return jsonOk({ data: { status: "ok" }, status: 201 });
}
}
4 changes: 1 addition & 3 deletions src/typegate/src/services/auth/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ export async function handleAuth(
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({ providers }, new Headers());
}

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 @@ import {
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 @@ export class OAuth2Auth extends Protocol {
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}`,
headers,
status: 400,
});
}
}
Expand Down Expand Up @@ -237,7 +239,8 @@ export class OAuth2Auth extends Protocol {
});
}

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 @@ export async function take(params: RouteParams) {
);

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,
});
}
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({
status: 401,
message: "not authorized",
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 });
}

type Action =
Expand Down Expand Up @@ -69,11 +67,11 @@ export async function validate(params: RouteParams): Promise<Response> {
}

// return json response
return new Response(JSON.stringify(result), {
headers: {
"content-type": "application/json",
return jsonOk({
data: result,
headers: new Headers({
"access-control-allow-origin": "*",
...headers,
},
}),
});
}
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: 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);
jsonError({ status: 400, message: e.message, headers });
}
}
}
28 changes: 23 additions & 5 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,
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