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

route error fix #18

Merged
merged 3 commits into from
Nov 6, 2024
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
61 changes: 0 additions & 61 deletions apps/api/app/api/[...route]/route.ts

This file was deleted.

61 changes: 61 additions & 0 deletions apps/api/app/api/[[...route]]/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { prisma } from "@repo/db";
import { Hono } from "hono";
import { handle } from "hono/vercel";


export const runtime = "nodejs";

export const app = new Hono().basePath("/api");
Comment on lines +1 to +8
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add database connection error handling.

Consider adding error handling for the Prisma client initialization to gracefully handle database connection issues.

 import { prisma } from "@repo/db";
 import { Hono } from "hono";
 import { handle } from "hono/vercel";
+
+// Middleware to handle database connection errors
+app.use("*", async (c, next) => {
+  try {
+    await next()
+  } catch (error) {
+    if (error instanceof PrismaClientKnownRequestError) {
+      return c.json({ error: "Database error occurred" }, 503)
+    }
+    throw error
+  }
+})

Committable suggestion skipped: line range outside the PR's diff.


app.get("/hello",async(c) => {
const test = await prisma.user.findMany()
return c.json({
test
});
})
Comment on lines +10 to +15
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add pagination and response type safety for GET /hello endpoint.

The endpoint returns all users without pagination, which could cause performance issues with large datasets.

-app.get("/hello",async(c) => {
+app.get("/hello", async (c) => {
+  const { page = 1, limit = 10 } = c.req.query();
   const test = await prisma.user.findMany({
+    take: Number(limit),
+    skip: (Number(page) - 1) * Number(limit),
+    select: {
+      id: true,
+      name: true,
+      // Add other fields you want to expose
+    }
   })
   return c.json({
-    test
+    data: test,
+    page: Number(page),
+    limit: Number(limit)
   });
 })
📝 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
app.get("/hello",async(c) => {
const test = await prisma.user.findMany()
return c.json({
test
});
})
app.get("/hello", async (c) => {
const { page = 1, limit = 10 } = c.req.query();
const test = await prisma.user.findMany({
take: Number(limit),
skip: (Number(page) - 1) * Number(limit),
select: {
id: true,
name: true,
// Add other fields you want to expose
}
})
return c.json({
data: test,
page: Number(page),
limit: Number(limit)
});
})

.patch( async(c) => {
const name = await c.req.json()
const test = await prisma.user.update({
where: {
id: "123",
},
data: {
name:name.name
}
})
return c.json({
test
});
})
Comment on lines +16 to +29
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Remove hardcoded ID and add proper error handling in PATCH endpoint.

The endpoint uses a hardcoded ID and lacks proper input validation.

-app.patch( async(c) => {
+app.patch("/:id", async (c) => {
   const name = await c.req.json()
+  const id = c.req.param('id')
+  
+  if (!name?.name || typeof name.name !== 'string') {
+    return c.json({ error: "Invalid name provided" }, 400)
+  }
+
   const test = await prisma.user.update({
     where: {
-      id: "123",
+      id: id,
     },
     data: {
       name:name.name
     }
   })
   return c.json({
-    test
+    data: test
   });
 })

Committable suggestion skipped: line range outside the PR's diff.

.delete( async(c) => {
const test = await prisma.user.delete({
where: {
id: "2",
}
})
return c.json({
test
});
})
Comment on lines +30 to +39
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Remove hardcoded ID and add proper error handling in DELETE endpoint.

Similar to the PATCH endpoint, this uses a hardcoded ID and lacks error handling.

-app.delete( async(c) => {
+app.delete("/:id", async (c) => {
+  const id = c.req.param('id')
+
+  try {
   const test = await prisma.user.delete({
     where: {
-      id: "2",
+      id: id,
     }
   })
   return c.json({
-    test
+    data: test
   });
+  } catch (error) {
+    if (error.code === 'P2025') {
+      return c.json({ error: "User not found" }, 404)
+    }
+    throw error
+  }
 })
📝 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
.delete( async(c) => {
const test = await prisma.user.delete({
where: {
id: "2",
}
})
return c.json({
test
});
})
.delete("/:id", async (c) => {
const id = c.req.param('id')
try {
const test = await prisma.user.delete({
where: {
id: id,
}
})
return c.json({
data: test
});
} catch (error) {
if (error.code === 'P2025') {
return c.json({ error: "User not found" }, 404)
}
throw error
}
})

.post(async(c) => {
const body = await c.req.json()
console.log(body)
const test = await prisma.user.create({
data: body
})
return c.json({
test
});
});
Comment on lines +40 to +49
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation and error handling for POST endpoint.

The endpoint lacks input validation and proper error handling.

 app.post(async(c) => {
   const body =  await c.req.json()
-  console.log(body)
+  
+  if (!body?.name || typeof body.name !== 'string') {
+    return c.json({ error: "Invalid user data" }, 400)
+  }
+
   const test = await prisma.user.create({
     data: body
   })
   return c.json({
-    test
+    data: test
   });
 });


app.get("/health", async(c) => {
return c.json({
message:"i am alive",
status:200
});
});

export const GET = handle(app);
export const POST = handle(app);
export const PATCH = handle(app);
export const DELETE = handle(app);
4 changes: 2 additions & 2 deletions apps/api/tests/hello.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { describe, it, expect, test } from "vitest";
import request from "supertest";
import { app } from "../app/api/[...route]/route";
import { app } from "../app/api/[[...route]]/route";

describe("Example", () => {
test("GET /health", async () => {
const res = await app.request("/api/health");
expect(res.status).toBe(200);
expect(await res.text()).toBe(
JSON.stringify({ message: "i am alive", status: 200 }),
JSON.stringify({ message: "i am alive", status: 200 })
);
});
});
Loading