From 261296a87406b3a427b85af951440c14c64c5e75 Mon Sep 17 00:00:00 2001 From: Aman Karn Date: Thu, 11 Jun 2026 18:52:45 +0000 Subject: [PATCH] fix: improve error handling across backend and frontend - Replace silent `if (!userId) return` with proper 401 JSON responses in user, issue, and repo controllers - Fix middleware silent `if(!secret) return` to send 500 responses instead of hanging the request - Make blacklistToken propagate errors (throw) instead of swallowing - Await blacklistToken calls in auth/user controllers - Make logoutUser handler async to support await - Fix deleteRepository re-throwing unhandled error (now returns 500) - Fix unstarRepository missing return after early response - Add userId null checks to getMyRepositories/getMyStarredRepos - Add try/catch and env var validation to copyRepoFilesInS3 - Fix CLI commands to set process.exitCode on failure instead of silently swallowing errors - Frontend useAuth: expose error state with API error message extraction - Frontend useUser: expose error state for consuming components Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- backend/src/controller/auth.controller.ts | 10 +-- backend/src/controller/command.controller.ts | 18 ++++-- backend/src/controller/issue.controller.ts | 30 +++++++-- backend/src/controller/repo.controller.ts | 16 ++++- backend/src/controller/user.controller.ts | 28 +++++++-- backend/src/middlewares/isAuthenticated.ts | 7 ++- backend/src/middlewares/optionalAuth.ts | 7 ++- backend/src/utils/blacklistToken.ts | 3 +- backend/src/utils/copyRepoFiles.ts | 64 +++++++++++--------- frontend/src/hooks/useAuth.ts | 36 ++++++++--- frontend/src/hooks/useUser.ts | 7 ++- 11 files changed, 160 insertions(+), 66 deletions(-) diff --git a/backend/src/controller/auth.controller.ts b/backend/src/controller/auth.controller.ts index 4a385aa..b5b9a7c 100644 --- a/backend/src/controller/auth.controller.ts +++ b/backend/src/controller/auth.controller.ts @@ -137,8 +137,8 @@ export const refreshAccessToken = async (req: Request, res: Response) => { }) } - blackListToken(decodeRefreshToken.jti, 604800); - blackListToken(decodeAccessToken.jti, 900); + await blackListToken(decodeRefreshToken.jti, 604800); + await blackListToken(decodeAccessToken.jti, 900); generateToken(decodeRefreshToken.id, res); @@ -156,7 +156,7 @@ export const refreshAccessToken = async (req: Request, res: Response) => { } -export const logoutUser = (req: Request, res: Response) => { +export const logoutUser = async (req: Request, res: Response) => { const refreshToken = req.cookies.refreshToken; const accessToken = req.cookies.accessToken; @@ -171,8 +171,8 @@ export const logoutUser = (req: Request, res: Response) => { const decodeAccessToken = jwt.verify(accessToken, process.env.JWT_SECRET!) as jwtPayload; //blacklist both tokens - blackListToken(decodeRefreshToken.jti, 604800); - blackListToken(decodeAccessToken.jti, 900); + await blackListToken(decodeRefreshToken.jti, 604800); + await blackListToken(decodeAccessToken.jti, 900); res.clearCookie("refreshToken"); res.clearCookie("accessToken"); diff --git a/backend/src/controller/command.controller.ts b/backend/src/controller/command.controller.ts index f2942b8..f9c51cb 100644 --- a/backend/src/controller/command.controller.ts +++ b/backend/src/controller/command.controller.ts @@ -69,7 +69,8 @@ export async function init(argv: any) { } } catch (error) { - console.log("Error in intialising the repository", error); + console.error("Error initializing the repository:", error); + process.exitCode = 1; } } @@ -96,8 +97,9 @@ export async function addRepo(argv: any) { if (error.code === 'ENOENT') { console.error(`Error: The file "${argv.file}" does not exist.`); } else { - console.error("Error adding file: ", error.message); + console.error("Error adding file:", error.message); } + process.exitCode = 1; } } @@ -156,7 +158,8 @@ export async function commitRepo(argv: ArgumentsCamelCase) { console.log(`Message: ${argv.message}`) } catch (error) { - console.log("Error in commiting: ", error); + console.error("Error in committing:", error); + process.exitCode = 1; } } @@ -198,7 +201,8 @@ export async function pushRepo() { console.log("All commits pushed to S3."); } catch (error) { - console.log("Error pushing to s3: ", error); + console.error("Error pushing to S3:", error); + process.exitCode = 1; } } @@ -251,7 +255,8 @@ export async function pullRepo() { console.log("All commits pulled from s3 successfully"); } catch (error) { - console.log("Error in pulling from s3: ", error); + console.error("Error pulling from S3:", error); + process.exitCode = 1; } } @@ -281,6 +286,7 @@ export async function revertRepo({commitId}: {commitId: string}) { console.log(`Commit ${commitId} reverted successfully.`); } catch (error) { - console.log("Unable to revert: ", error); + console.error("Unable to revert:", error); + process.exitCode = 1; } } \ No newline at end of file diff --git a/backend/src/controller/issue.controller.ts b/backend/src/controller/issue.controller.ts index 6924317..6fb73df 100644 --- a/backend/src/controller/issue.controller.ts +++ b/backend/src/controller/issue.controller.ts @@ -21,7 +21,11 @@ export const createIssue = async (req: Request, res: Response) => { } const userId = req.user?.id; - if(!userId) return; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const ownerName = parsedParams.data.owner; @@ -203,7 +207,11 @@ export const getIssueById = async (req: Request, res: Response) => { export const updateIssue = async (req: Request, res: Response) => { const userId = req.user?.id; - if(!userId) return; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } const parsedParams = issueByIdSchema.safeParse(req.params); if(!parsedParams.success) { @@ -281,7 +289,11 @@ export const updateIssue = async (req: Request, res: Response) => { export const deleteIssue = async (req: Request, res: Response) => { const userId = req.user?.id; - if(!userId) return; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } const parsedParams = issueByIdSchema.safeParse(req.params); if(!parsedParams.success) { @@ -327,7 +339,11 @@ export const deleteIssue = async (req: Request, res: Response) => { //own issues export const getMyIssues = async (req: Request, res: Response) => { const userId = req.user?.id; - if(!userId) return; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const issuesAcrossAllRepos = await prismaClient.issue.findMany({ @@ -378,7 +394,11 @@ export const updateMyIssuesById = async (req: Request, res: Response) => { } const userId = req.user?.id; - if(!userId) return; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } const parsedData = issueUpdateSchema.safeParse(req.body); if(!parsedData.success) { diff --git a/backend/src/controller/repo.controller.ts b/backend/src/controller/repo.controller.ts index dbb2d4b..65b7e4d 100644 --- a/backend/src/controller/repo.controller.ts +++ b/backend/src/controller/repo.controller.ts @@ -240,6 +240,11 @@ export const getRepositoryByFullName = async (req: Request, res: Response) => { //Authenticated Operations export const getMyRepositories = async (req: Request, res: Response) => { const userId = req.user?.id; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const userReopos = await prismaClient.user.findUnique({ @@ -279,6 +284,11 @@ export const getMyRepositories = async (req: Request, res: Response) => { export const getMyStarredRepos = async (req: Request, res: Response) => { const userId = req.user?.id; + if(!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const userStarredReopos = await prismaClient.user.findUnique({ @@ -471,7 +481,9 @@ export const deleteRepository = async (req: Request, res: Response) => { if (error.code === 'P2025') { //Prisma's "record not found" error code return res.status(404).json({ message: "Repo not found" }) } - throw error + res.status(500).json({ + message: "Error in server" + }) } } @@ -651,7 +663,7 @@ export const unstarRepository = async (req: Request, res: Response) => { }) } if(targetRepo.staredBy.length === 0) { - res.status(400).json({ + return res.status(400).json({ message: "You haven't starred this repo" }) } diff --git a/backend/src/controller/user.controller.ts b/backend/src/controller/user.controller.ts index 771aba1..132b0d8 100644 --- a/backend/src/controller/user.controller.ts +++ b/backend/src/controller/user.controller.ts @@ -55,7 +55,11 @@ export const getCurrentUser = async (req: Request, res: Response) => { export const updateUserProfile = async (req: Request, res: Response) => { const userId = req.user?.id; - if (!userId) return; + if (!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } const parsedData = userUpdateSchema.safeParse(req.body); if (!parsedData.success) { @@ -147,7 +151,11 @@ export const deleteUserProfile = async (req: Request, res: Response) => { const refreshToken = req.cookies.refreshToken; const accessToken = req.cookies.accessToken; const userId = req.user?.id; //from cookies - if (!userId) return; + if (!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const user = await prismaClient.user.delete({ @@ -160,8 +168,8 @@ export const deleteUserProfile = async (req: Request, res: Response) => { const decodeAccessToken = jwt.verify(accessToken, process.env.JWT_SECRET!) as jwtPayload; //blacklist both tokens - blackListToken(decodeRefreshToken.jti, 604800); - blackListToken(decodeAccessToken.jti, 900); + await blackListToken(decodeRefreshToken.jti, 604800); + await blackListToken(decodeAccessToken.jti, 900); res.clearCookie("refreshToken"); res.clearCookie("accessToken"); @@ -181,7 +189,11 @@ export const deleteUserProfile = async (req: Request, res: Response) => { export const getMyFollowers = async (req: Request, res: Response) => { const userId = req.user?.id; - if (!userId) return; + if (!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { //following me @@ -216,7 +228,11 @@ export const getMyFollowers = async (req: Request, res: Response) => { export const getMyFollowing = async (req: Request, res: Response) => { const userId = req.user?.id; - if (!userId) return; + if (!userId) { + return res.status(401).json({ + message: "Unauthorized" + }) + } try { const user = await prismaClient.user.findUnique({ diff --git a/backend/src/middlewares/isAuthenticated.ts b/backend/src/middlewares/isAuthenticated.ts index a938646..60a74f2 100644 --- a/backend/src/middlewares/isAuthenticated.ts +++ b/backend/src/middlewares/isAuthenticated.ts @@ -21,7 +21,12 @@ export default async function isAuthenticated(req: Request, res: Response, next: } const secret = process.env.JWT_SECRET; - if(!secret) return; + if(!secret) { + console.error("JWT_SECRET environment variable is not configured"); + return res.status(500).json({ + message: "Server configuration error" + }); + } try { const decodeAccessToken = jwt.verify(accessToken, secret) as jwtPayload; diff --git a/backend/src/middlewares/optionalAuth.ts b/backend/src/middlewares/optionalAuth.ts index 77103b4..74a418f 100644 --- a/backend/src/middlewares/optionalAuth.ts +++ b/backend/src/middlewares/optionalAuth.ts @@ -5,7 +5,12 @@ import type { jwtPayload } from "../utils/generateToken.js"; export default async function optionalAuth(req: Request, res: Response, next: NextFunction) { const accessToken = req.cookies.accessToken; const secret = process.env.JWT_SECRET; - if(!secret) return; + if(!secret) { + console.error("JWT_SECRET environment variable is not configured"); + return res.status(500).json({ + message: "Server configuration error" + }); + } if(!accessToken) { return next(); diff --git a/backend/src/utils/blacklistToken.ts b/backend/src/utils/blacklistToken.ts index 3954455..602749d 100644 --- a/backend/src/utils/blacklistToken.ts +++ b/backend/src/utils/blacklistToken.ts @@ -7,6 +7,7 @@ export default async function blackListToken(token: string, expiryInSeconds: num EX: expiryInSeconds }); } catch (error) { - console.log("Error in token blacklisting funtion", error) + console.error("Error in token blacklisting function:", error); + throw new Error("Failed to blacklist token"); } } \ No newline at end of file diff --git a/backend/src/utils/copyRepoFiles.ts b/backend/src/utils/copyRepoFiles.ts index 5ca236b..e66a2d0 100644 --- a/backend/src/utils/copyRepoFiles.ts +++ b/backend/src/utils/copyRepoFiles.ts @@ -9,40 +9,50 @@ import { s3 } from "../config/aws.config.js"; export default async function copyRepoFilesInS3(originalRepoId: string, forkedByUserId: string) { const prefix = `repos/${originalRepoId}/commits/`; const S3_BUCKET = process.env.S3_BUCKET; - const forkedRepoId = `${originalRepoId}_${forkedByUserId}` + if(!S3_BUCKET) { + throw new Error("S3_BUCKET environment variable is not configured"); + } - //get all files from s3 - const listCommad = new ListObjectsV2Command({ - Bucket: S3_BUCKET, - Prefix: prefix, - }) + const forkedRepoId = `${originalRepoId}_${forkedByUserId}` - const listed = await s3.send(listCommad); + try { + //get all files from s3 + const listCommad = new ListObjectsV2Command({ + Bucket: S3_BUCKET, + Prefix: prefix, + }) - if(!listed.Contents || listed.Contents?.length === 0){ - console.log("Original repo has no pushed commits to copy."); - return forkedRepoId; - } + const listed = await s3.send(listCommad); - for(const obj of listed.Contents) { - if(!obj.Key) { - continue; + if(!listed.Contents || listed.Contents?.length === 0){ + console.log("Original repo has no pushed commits to copy."); + return forkedRepoId; } - const newKey = obj.Key.replace( - `repos/${originalRepoId}/`, `repos/${originalRepoId}_${forkedByUserId}/` - ) + for(const obj of listed.Contents) { + if(!obj.Key) { + continue; + } - //this command will copy in the new created forked repo - const copyCommand = new CopyObjectCommand({ - Bucket: S3_BUCKET, - CopySource: `${S3_BUCKET}/${obj.Key}`, - Key: newKey, - }) + const newKey = obj.Key.replace( + `repos/${originalRepoId}/`, `repos/${originalRepoId}_${forkedByUserId}/` + ) - await s3.send(copyCommand); - } + //this command will copy in the new created forked repo + const copyCommand = new CopyObjectCommand({ + Bucket: S3_BUCKET, + CopySource: `${S3_BUCKET}/${obj.Key}`, + Key: newKey, + }) + + await s3.send(copyCommand); + } - console.log("S3 fiels copied to forked path."); - return forkedRepoId; + console.log("S3 files copied to forked path."); + return forkedRepoId; + + } catch (error) { + console.error("Error copying repo files in S3:", error); + throw new Error("Failed to copy repository files in S3"); + } } \ No newline at end of file diff --git a/frontend/src/hooks/useAuth.ts b/frontend/src/hooks/useAuth.ts index 2a91d37..d70f9d6 100644 --- a/frontend/src/hooks/useAuth.ts +++ b/frontend/src/hooks/useAuth.ts @@ -1,6 +1,7 @@ -import { useContext } from "react"; +import { useContext, useState } from "react"; import { AuthContext } from "../context/AuthContext"; import { loginApi, logoutApi, registerApi } from "../api/auth.api"; +import axios from "axios"; interface Register { @@ -15,6 +16,16 @@ interface Login { password: string } +function extractErrorMessage(error: unknown, fallback: string): string { + if (axios.isAxiosError(error)) { + return error.response?.data?.message ?? fallback; + } + if (error instanceof Error) { + return error.message; + } + return fallback; +} + export function useAuth() { const context = useContext(AuthContext); if(!context) { @@ -22,17 +33,20 @@ export function useAuth() { } const { user, setUser, loading, setLoading } = context; + const [error, setError] = useState(null); const handleLogin = async ({username, password}: Login) => { setLoading(true); + setError(null); try { const data = await loginApi({username, password}); setUser(data.user); return true; - } catch { - console.log("Client side error in login function"); + } catch (err) { + const message = extractErrorMessage(err, "Login failed"); + setError(message); return false; } finally { setLoading(false); @@ -41,16 +55,16 @@ export function useAuth() { const handleRegister = async ({ username, email, name, password }: Register) => { setLoading(true); + setError(null); try { const data = await registerApi({username, email, name, password}); - console.log(data); - setUser(data.data); return true; - } catch { - console.log("Client side error in regeistre function"); + } catch (err) { + const message = extractErrorMessage(err, "Registration failed"); + setError(message); return false; } finally { setLoading(false); @@ -59,18 +73,20 @@ export function useAuth() { const handleLogout = async () => { setLoading(true); + setError(null); try { await logoutApi(); setUser(null); return true; - } catch { - console.log("Client side error in logout function"); + } catch (err) { + const message = extractErrorMessage(err, "Logout failed"); + setError(message); return false; } finally { setLoading(false); } } - return { user, loading, handleLogin, handleRegister, handleLogout } + return { user, loading, error, handleLogin, handleRegister, handleLogout } } \ No newline at end of file diff --git a/frontend/src/hooks/useUser.ts b/frontend/src/hooks/useUser.ts index 5ce5685..d4dcf53 100644 --- a/frontend/src/hooks/useUser.ts +++ b/frontend/src/hooks/useUser.ts @@ -45,11 +45,13 @@ export interface UserProfile { export default function useUser(username?: string, isCurrentUser: boolean = false) { const [user, setUser] = useState(null); const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); const {user: currentUser} = useAuth(); useEffect(() => { const fetchUserData = async () => { setLoading(true); + setError(null); try { let userData: UserProfile | null = null; @@ -81,7 +83,8 @@ export default function useUser(username?: string, isCurrentUser: boolean = fals setUser(userData); } catch (error) { - console.log('Error in useUser: ', error); + const message = error instanceof Error ? error.message : "Failed to load user profile"; + setError(message); } finally { setLoading(false); } @@ -90,5 +93,5 @@ export default function useUser(username?: string, isCurrentUser: boolean = fals fetchUserData(); }, [username, isCurrentUser, currentUser]); - return { user, loading } + return { user, loading, error } } \ No newline at end of file