Skip to content

Add consistent JSON error responses with structured error codes #10

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.

Sign up for GitHub

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

Closed
DominicBM wants to merge 1 commit into from
Closed

Add consistent JSON error responses with structured error codes #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 33 additions & 25 deletions src/aggregation/responses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,56 +1,64 @@
/**
* It's not you, it's us.
*/
export class FiveHundredResponse {
constructor(message: string, errorCode: number) {
class ErrorResponse {
constructor(
message: string,
readonly errorCode: number,
error: string,
) {
this.message = message;
this.errorCode = errorCode;
this.error = error;
}

message: string;
errorCode: number;
}
error: string;

export class InternalErrorResponse extends FiveHundredResponse {
constructor() {
super("Internal error", 500);
toJSON() {
return { error: this.error, message: this.message };
}
}

/**
* It's not us, it's you.
*/
export class FourHundredResponse {
constructor(message: string, errorCode: number) {
this.message = message;
this.errorCode = errorCode;
}

message: string;
errorCode: number;
}
export class FourHundredResponse extends ErrorResponse {}

export class InvalidEmail extends FourHundredResponse {
constructor() {
super("Invalid email address.", 400);
super("Invalid email address.", 400, "invalid_email");
}
}

export class UnrecognizedParameters extends FourHundredResponse {
constructor(message: string) {
super("Unrecognized parameters: " + message, 400);
super("Unrecognized parameters: " + message, 400, "unrecognized_parameters");
}
}

export class InvalidParameter extends FourHundredResponse {
constructor(message: string) {
super("Invalid parameter: " + message, 400);
super("Invalid parameter: " + message, 400, "invalid_parameter");
}
}

export class TooManyIdentifiers extends FourHundredResponse {
constructor(message: string) {
super(message, 400);
super(message, 400, "too_many_identifiers");
}
}

export class UnauthorizedResponse extends FourHundredResponse {
constructor() {
super("Unauthorized", 401, "unauthorized");
}
}

/**
* It's not you, it's us.
*/
export class FiveHundredResponse extends ErrorResponse {}

export class InternalErrorResponse extends FiveHundredResponse {
constructor() {
super("Internal error", 500, "internal_error");
}
}

Expand Down Expand Up @@ -82,7 +90,7 @@ interface Facet {
field: string;
type: string;
buckets: Bucket[];
bucketsLabel: String;
bucketsLabel: string;
}

interface Bucket {
Expand Down
20 changes: 10 additions & 10 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
DPLADocList,
FourHundredResponse,
FiveHundredResponse,
UnauthorizedResponse,
EmailSent,
} from "./aggregation/responses";
import ApiKeyRepository from "./aggregation/api_key_repository";
Expand Down Expand Up @@ -115,12 +116,14 @@ function worker() {
}

if (!apiKeyRepository.isApiKeyValid(apiKey)) {
return res.status(401).json({ message: "Unauthorized" });
const r = new UnauthorizedResponse();
return res.status(r.errorCode).json(r);
}
const user = await apiKeyRepository.findUserByApiKey(apiKey);

if (!user) {
return res.status(401).json({ message: "Unauthorized" });
const r = new UnauthorizedResponse();
return res.status(r.errorCode).json(r);
}

next();
Expand All @@ -136,14 +139,11 @@ function worker() {

const queryParams = (req: express.Request): Map<string, string> => {
const params = new Map<string, string>();
for (const key in Object.entries(req.query)) {
if (req.query.hasOwnProperty(key)) {
const value = req.query[key];
if (typeof value === "string") {
params.set(key, value);
} else if (Array.isArray(value)) {
params.set(key, value[0] as string);
}
for (const [key, value] of Object.entries(req.query)) {
if (typeof value === "string") {
params.set(key, value);
} else if (Array.isArray(value)) {
params.set(key, value[0] as string);
}
Comment on lines +142 to 147
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard array query values before writing to Map<string, string>.

On Line 146, value[0] as string can write undefined (empty array) or non-string values. Add a runtime type check before params.set.

🛠️ Proposed fix 
     for (const [key, value] of Object.entries(req.query)) {
       if (typeof value === "string") {
         params.set(key, value);
       } else if (Array.isArray(value)) {
-        params.set(key, value[0] as string);
+        const first = value[0];
+        if (typeof first === "string") {
+          params.set(key, first);
+        }
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const [key, value] of Object.entries(req.query)) {
if (typeof value === "string") {
params.set(key, value);
} else if (Array.isArray(value)) {
params.set(key, value[0] as string);
}
for (const [key, value] of Object.entries(req.query)) {
if (typeof value === "string") {
params.set(key, value);
} else if (Array.isArray(value)) {
const first = value[0];
if (typeof first === "string") {
params.set(key, first);
}
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 142 - 147, The loop over req.query is writing
array entries into the params Map<string,string> without checking array
contents; update the Array.isArray branch (where params.set(key, value[0] as
string) is used) to first ensure value.length > 0 and typeof value[0] ===
"string" (or coerce safely with String(value[0]) if you intend to accept
non-strings) before calling params.set; otherwise skip the key (or log/handle
the empty/non-string case) so you never insert undefined or invalid types into
params.

}
return params;
Expand Down
Loading