Skip to content

Commit 82600b5

Browse files
authored
fix: Use GraphQL-compliant response payload on error (#972)
<!-- Pull requests are squashed and merged using: - their title as the commit message - their description as the commit body Having a good title and description is important for the users to get readable changelog. --> <!-- 1. Explain WHAT the change is about --> - Use GraphQL-compliant response payload on error on all GraphQL endpoints <!-- 2. Explain WHY the change cannot be made simpler --> <!-- 3. Explain HOW users should update their code --> #### Migration notes --- - [ ] The change comes with new or modified tests - [ ] Hard-to-understand functions have explanatory comments - [ ] End-user documentation is updated to reflect the change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Consolidated JSON response formatting for both success and error cases. - Streamlined the handling of response structures across multiple service endpoints. - Improved consistency and reliability of data returned to clients, ensuring structured JSON output for all interactions. - Enhanced error handling to provide structured JSON responses across various services. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent f027067 commit 82600b5

File tree

8 files changed

+71
-77
lines changed

8 files changed

+71
-77
lines changed

src/typegate/src/services/artifact_service.ts

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
import { z } from "zod";
1010
import { getLogger } from "../log.ts";
1111
import { BaseError, UnknownError } from "../errors.ts";
12+
import { jsonError } from "./responses.ts";
13+
import { jsonOk } from "./responses.ts";
1214

1315
const logger = getLogger(import.meta);
1416

@@ -25,9 +27,9 @@ export class ArtifactService {
2527
if (operation === "prepare-upload") {
2628
if (request.method !== "POST") {
2729
logger.warn("Method not allowed: {}", request.method);
28-
return new Response(JSON.stringify({ error: "method not allowed" }), {
30+
return jsonError({
31+
message: `method not allowed: ${request.method}`,
2932
status: 405,
30-
headers: { "Content-Type": "application/json" },
3133
});
3234
}
3335

@@ -36,20 +38,15 @@ export class ArtifactService {
3638
metaList = prepareUploadBodySchema.parse(await request.json());
3739
} catch (error) {
3840
logger.error("Failed to parse data: {}", error);
39-
return new Response(
40-
JSON.stringify({ error: `Invalid Request Body: ${error.message}` }),
41-
{
42-
status: 400,
43-
headers: { "Content-Type": "application/json" },
44-
},
45-
);
41+
return jsonError({
42+
message: `invalid request body: ${error.message}`,
43+
status: 400,
44+
});
4645
}
4746

4847
try {
4948
const data = await this.#createUploadTokens(metaList, tgName);
50-
return new Response(JSON.stringify(data), {
51-
headers: { "Content-Type": "application/json" },
52-
});
49+
return jsonOk({ data });
5350
} catch (e) {
5451
if (e instanceof BaseError) {
5552
return e.toResponse();
@@ -60,28 +57,22 @@ export class ArtifactService {
6057

6158
if (operation) {
6259
logger.warn("not found: {} {}", request.method, url.toString());
63-
return new Response(JSON.stringify({ message: "not found" }), {
64-
status: 404,
65-
headers: { "Content-Type": "application/json" },
66-
});
60+
return jsonError({ message: "not found", status: 404 });
6761
}
6862

6963
if (request.method !== "POST") {
7064
logger.warn("Method not allowed: {}", request.method);
71-
return new Response(JSON.stringify({ error: "method not allowed" }), {
65+
return jsonError({
66+
message: `method not allowed: ${request.method}`,
7267
status: 405,
73-
headers: { "Content-Type": "application/json" },
7468
});
7569
}
7670

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

7973
if (!token) {
8074
logger.warn("Missing upload token");
81-
return new Response(JSON.stringify({ error: "missing token" }), {
82-
status: 403,
83-
headers: { "Content-Type": "application/json" },
84-
});
75+
return jsonError({ message: "missing token", status: 403 });
8576
}
8677

8778
return await this.#handleUpload(token, request.body!, tgName);
@@ -110,10 +101,7 @@ export class ArtifactService {
110101
if (e instanceof BaseError) {
111102
return e.toResponse();
112103
}
113-
return new Response(JSON.stringify({ error: e.message }), {
114-
status: 500,
115-
headers: { "Content-Type": "application/json" },
116-
});
104+
return jsonError({ message: e.message, status: 500 });
117105
}
118106

119107
if (meta.typegraphName !== tgName) {
@@ -125,17 +113,9 @@ export class ArtifactService {
125113
if (hash !== meta.hash) {
126114
await this.store.persistence.delete(hash);
127115
logger.warn("hash mismatch: {} {}", hash, meta.hash);
128-
return new Response(JSON.stringify({ error: "hash mismatch" }), {
129-
status: 403,
130-
headers: { "Content-Type": "application/json" },
131-
});
116+
return jsonError({ message: "hash mismatch", status: 403 });
132117
}
133118

134-
return new Response(JSON.stringify({ status: "ok" }), {
135-
status: 201,
136-
headers: {
137-
"Content-Type": "application/json",
138-
},
139-
});
119+
return jsonOk({ data: { status: "ok" }, status: 201 });
140120
}
141121
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { unsafeExtractJWT } from "../../crypto.ts";
1414
import type { QueryEngine } from "../../engine/query_engine.ts";
1515
import * as routes from "./routes/mod.ts";
1616
import { getLogger } from "../../log.ts";
17-
import { methodNotAllowed } from "../../services/responses.ts";
17+
import { jsonOk, methodNotAllowed } from "../../services/responses.ts";
1818
import type { Runtime } from "../../runtimes/Runtime.ts";
1919

2020
const logger = getLogger(import.meta);
@@ -132,9 +132,7 @@ export async function handleAuth(
132132
uri:
133133
`${url.protocol}//${url.host}/${engine.name}/auth/${name}?redirect_uri=${origin}`,
134134
}));
135-
return new Response(JSON.stringify({ providers }), {
136-
headers: { "content-type": "application/json" },
137-
});
135+
return jsonOk({ data: providers });
138136
}
139137

140138
return await provider.authMiddleware(request);

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
generateWeakValidator,
2727
} from "../../../engine/typecheck/input.ts";
2828
import type { TokenMiddlewareOutput } from "./protocol.ts";
29+
import { jsonError } from "../../responses.ts";
2930

3031
const logger = getLogger(import.meta);
3132

@@ -205,9 +206,10 @@ export class OAuth2Auth extends Protocol {
205206
new Headers(),
206207
);
207208
// https://github.com/cmd-johnson/deno-oauth2-client/issues/25
208-
return new Response(`invalid oauth2, check your credentials: ${e}`, {
209-
status: 400,
209+
return jsonError({
210+
message: `invalid oauth2, check your credentials: ${e}`,
210211
headers,
212+
status: 400,
211213
});
212214
}
213215
}
@@ -237,7 +239,8 @@ export class OAuth2Auth extends Protocol {
237239
});
238240
}
239241

240-
return new Response("missing origin or redirect_uri query parameter", {
242+
return jsonError({
243+
message: "missing origin or redirect_uri query parameter",
241244
status: 400,
242245
});
243246
}

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: MPL-2.0
33

44
import { getLogger } from "../../../log.ts";
5+
import { jsonError, jsonOk } from "../../responses.ts";
56
import { clearCookie, getEncryptedCookie } from "../cookies.ts";
67
import type { RouteParams } from "./mod.ts";
78

@@ -20,23 +21,19 @@ export async function take(params: RouteParams) {
2021
);
2122

2223
if (!redirectUri.startsWith(origin)) {
23-
return new Response(
24-
"take request must share domain with redirect uri",
25-
{
26-
status: 400,
27-
headers: resHeaders,
28-
},
29-
);
24+
return jsonError({
25+
status: 400,
26+
message: "take request must share domain with redirect uri",
27+
headers: resHeaders,
28+
});
3029
}
3130
resHeaders.set("content-type", "application/json");
32-
return new Response(JSON.stringify({ token }), {
33-
status: 200,
34-
headers: resHeaders,
35-
});
31+
return jsonOk({ data: { token }, headers: resHeaders });
3632
} catch (e) {
3733
logger.info(`take request failed ${e}`);
38-
return new Response("not authorized", {
34+
return jsonError({
3935
status: 401,
36+
message: "not authorized",
4037
headers: resHeaders,
4138
});
4239
}

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
// Copyright Metatype OÜ, licensed under the Mozilla Public License Version 2.0.
22
// SPDX-License-Identifier: MPL-2.0
33

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

67
export function badRequest(message: string): Response {
7-
return new Response(message, {
8-
status: 400,
9-
headers: { "content-type": "text/plain" },
10-
});
8+
return jsonError({ status: 400, message });
119
}
1210

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

7169
// return json response
72-
return new Response(JSON.stringify(result), {
73-
headers: {
74-
"content-type": "application/json",
70+
return jsonOk({
71+
data: result,
72+
headers: new Headers({
7573
"access-control-allow-origin": "*",
7674
...headers,
77-
},
75+
}),
7876
});
7977
}

src/typegate/src/services/graphql_service.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,30 +119,30 @@ export async function handleGraphQL(
119119
);
120120
}
121121

122-
return jsonOk(res, headers);
122+
return jsonOk({ data: { data: res }, headers });
123123
} catch (e) {
124124
// throw e;
125125
if (e instanceof BaseError) {
126126
return e.toResponse(headers);
127127
}
128128
if (e instanceof ResolverError) {
129129
logger.error(`field err: ${e.message}`);
130-
return jsonError(e.message, headers, 502);
130+
return jsonError({ status: 502, message: e.message, headers });
131131
} else if (e instanceof BadContext) {
132132
logger.error(`context err: ${e.message}`);
133-
return jsonError(
134-
e.message,
133+
return jsonError({
134+
status: Object.keys(context).length === 0 ? 401 : 403,
135+
message: e.message,
135136
headers,
136-
Object.keys(context).length === 0 ? 401 : 403,
137-
);
137+
});
138138
} else {
139139
logger.error(`request err: ${Deno.inspect(e)}`);
140140
if (e.cause) {
141141
logger.error(
142142
Deno.inspect(e.cause, { strAbbreviateSize: 1024, depth: 10 }),
143143
);
144144
}
145-
return jsonError(e.message, headers, 400);
145+
return jsonError({ status: 400, message: e.message, headers });
146146
}
147147
}
148148
}

src/typegate/src/services/responses.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,37 @@
44
import type { JSONValue } from "../utils.ts";
55
import { BaseError, ErrorKind } from "../errors.ts";
66

7-
export const jsonOk = (data: JSONValue, headers: Headers) => {
7+
export type JsonOkConfig = {
8+
data: JSONValue;
9+
headers?: Headers | HeadersInit;
10+
status?: number;
11+
};
12+
13+
export const jsonOk = (
14+
{ status = 200, data, headers: headersInit }: JsonOkConfig,
15+
) => {
16+
const headers = headersInit != null
17+
? new Headers(headersInit)
18+
: new Headers();
819
headers.set("content-type", "application/json");
9-
return new Response(JSON.stringify({ data }), {
10-
status: 200,
20+
return new Response(JSON.stringify(data), {
21+
status,
1122
headers,
1223
});
1324
};
1425

26+
export type JsonErrorConfig = {
27+
status: number;
28+
message: string;
29+
headers?: Headers | HeadersInit;
30+
};
31+
1532
export const jsonError = (
16-
message: string,
17-
headers: Headers,
18-
status: number,
33+
{ status, message, headers: headersInit }: JsonErrorConfig,
1934
) => {
35+
const headers = headersInit != null
36+
? new Headers(headersInit)
37+
: new Headers();
2038
headers.set("content-type", "application/json");
2139
return new Response(
2240
JSON.stringify({

src/typegate/src/typegate/mod.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export class Typegate implements AsyncDisposable {
285285
new Headers(cors),
286286
).catch((e) => e);
287287
if (jwtCheck instanceof Error) {
288-
return jsonError(jwtCheck.message, new Headers(), 401);
288+
return jsonError({ message: jwtCheck.message, status: 401 });
289289
}
290290

291291
const [context, headers] = jwtCheck;

0 commit comments

Comments
 (0)