fix(core): Centralize server error handling and secure startup#1100
Open
Aathish101 wants to merge 1 commit intodigitomize:mainfrom
Open
fix(core): Centralize server error handling and secure startup#1100Aathish101 wants to merge 1 commit intodigitomize:mainfrom
Aathish101 wants to merge 1 commit intodigitomize:mainfrom
Conversation
👷 Deploy request for digitomize pending review.Visit the deploys page to approve it
|
👷 Deploy request for v2-digitomize pending review.Visit the deploys page to approve it
|
Author
|
updates |
Contributor
WalkthroughIntroduces a new Express server bootstrap at backend/chore/index.js with environment-driven initialization, MongoDB connection, route registration, periodic tasks, and centralized error handling. Removes the legacy server bootstrap at backend/index.js, including its routes, background syncers, and startup logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Env as Environment
participant App as Server Bootstrap
participant DB as MongoDB
participant Routes as Routes (Users/Contests/Extensions/Community/Hackathons)
participant Sched as Periodic Tasks
participant HTTP as HTTP Server
participant Err as Error Middleware
Env->>App: Read config (NODE_ENV, PORT, flags)
App->>DB: Connect
DB-->>App: Connected / Error
App->>Routes: Register routes
App->>Sched: Start syncers/validators/heartbeats
App->>HTTP: Listen(PORT)
HTTP-->>Env: Startup log/ready
note over App,HTTP: Production vs Development startup branches
alt Request handled
HTTP->>Routes: Incoming request
Routes-->>HTTP: Response
else Not Found
HTTP->>Err: 404 handler
Err-->>HTTP: 404 response
else Route Error
Routes-->>Err: Throw/next(err)
Err-->>HTTP: Error response
end
par Process-level resilience
App-)App: Handle uncaughtException
App-)App: Handle unhandledRejection
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/chore: Update index.js for stability and error logging(1 hunks)backend/index.js(0 hunks)
💤 Files with no reviewable changes (1)
- backend/index.js
Comment on lines
+55
to
+63
| console.log(process.env.TEST); | ||
| async function main() { | ||
| try { | ||
| console.log("Pinging..."); | ||
| await fetchContestsData(); | ||
| console.log("Pong!"); | ||
| } catch (error) { | ||
| console.error("Error pinging the server:", error); | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the environment-variable debug log.
Logging process.env.TEST in production dumps environment content to stdout, which can leak secrets in aggregated logs. Please drop this statement.
-console.log(process.env.TEST);📝 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
| console.log(process.env.TEST); | |
| async function main() { | |
| try { | |
| console.log("Pinging..."); | |
| await fetchContestsData(); | |
| console.log("Pong!"); | |
| } catch (error) { | |
| console.error("Error pinging the server:", error); | |
| } | |
| async function main() { | |
| try { | |
| console.log("Pinging..."); | |
| await fetchContestsData(); | |
| console.log("Pong!"); | |
| } catch (error) { | |
| console.error("Error pinging the server:", error); | |
| } |
🤖 Prompt for AI Agents
In backend/chore: Update index.js for stability and error logging around lines
55 to 63, remove the debug statement that prints environment variables (delete
the line console.log(process.env.TEST);) to avoid leaking secrets to logs; if
you need local debug visibility, gate a non-sensitive message behind a
development-only flag (e.g., if (process.env.NODE_ENV === 'development')
console.log('debug')); otherwise simply delete the env log.
Comment on lines
+97
to
+135
| await dataSyncer.syncContests(); | ||
| setInterval(dataSyncer.syncContests, 90 * 60 * 1000); | ||
|
|
||
| // Update contests data and sync contests data at regular intervals | ||
| await contestSyncer.updateContests(); | ||
| setInterval(contestSyncer.updateContests, 60 * 60 * 1000); | ||
|
|
||
| // Pinging the server every 13 minutes | ||
| setInterval( | ||
| async () => { | ||
| try { | ||
| await main(); | ||
| console.log("<=======Sent GET request to AWAKE"); | ||
| } catch (error) { | ||
| console.error("Error Pinging", error); | ||
| } | ||
| }, | ||
| 13 * 60 * 1000, | ||
| ); | ||
|
|
||
| // Set up contest routes | ||
| app.use("/contests", contestRoutes); | ||
| } | ||
|
|
||
| async function setupCommunityServer() { | ||
| app.use("/community", communityRoutes); | ||
| } | ||
|
|
||
| async function setupHackathonServer() { | ||
| await hackathonAPISyncer.syncHackathons(); | ||
| setInterval(hackathonAPISyncer.syncHackathons, 90 * 60 * 1000); | ||
|
|
||
| // Update contests data and sync contests data at regular intervals | ||
| await hackathonDBSyncer.updateHackathons(); | ||
| setInterval(hackathonDBSyncer.updateHackathons, 60 * 60 * 1000); | ||
|
|
||
| // Set up hackathon routes | ||
| app.use("/hackathons", hackathonRoutes); | ||
| } |
Contributor
There was a problem hiding this comment.
Handle async interval failures explicitly.
setInterval invoked with async functions (syncContests, updateContests, syncHackathons, updateHackathons) drops the returned promise. Any rejection becomes an unhandled promise rejection, which your global handler turns into a process exit. Wrap each interval body in an async arrow and catch/log failures so a transient sync error doesn’t take down the server.
- setInterval(dataSyncer.syncContests, 90 * 60 * 1000);
+ setInterval(async () => {
+ try {
+ await dataSyncer.syncContests();
+ } catch (err) {
+ console.error("Contest sync failed:", err);
+ }
+ }, 90 * 60 * 1000);
-
- setInterval(contestSyncer.updateContests, 60 * 60 * 1000);
+ setInterval(async () => {
+ try {
+ await contestSyncer.updateContests();
+ } catch (err) {
+ console.error("Contest update failed:", err);
+ }
+ }, 60 * 60 * 1000);
@@
- setInterval(hackathonAPISyncer.syncHackathons, 90 * 60 * 1000);
+ setInterval(async () => {
+ try {
+ await hackathonAPISyncer.syncHackathons();
+ } catch (err) {
+ console.error("Hackathon API sync failed:", err);
+ }
+ }, 90 * 60 * 1000);
-
- setInterval(hackathonDBSyncer.updateHackathons, 60 * 60 * 1000);
+ setInterval(async () => {
+ try {
+ await hackathonDBSyncer.updateHackathons();
+ } catch (err) {
+ console.error("Hackathon DB update failed:", err);
+ }
+ }, 60 * 60 * 1000);📝 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
| await dataSyncer.syncContests(); | |
| setInterval(dataSyncer.syncContests, 90 * 60 * 1000); | |
| // Update contests data and sync contests data at regular intervals | |
| await contestSyncer.updateContests(); | |
| setInterval(contestSyncer.updateContests, 60 * 60 * 1000); | |
| // Pinging the server every 13 minutes | |
| setInterval( | |
| async () => { | |
| try { | |
| await main(); | |
| console.log("<=======Sent GET request to AWAKE"); | |
| } catch (error) { | |
| console.error("Error Pinging", error); | |
| } | |
| }, | |
| 13 * 60 * 1000, | |
| ); | |
| // Set up contest routes | |
| app.use("/contests", contestRoutes); | |
| } | |
| async function setupCommunityServer() { | |
| app.use("/community", communityRoutes); | |
| } | |
| async function setupHackathonServer() { | |
| await hackathonAPISyncer.syncHackathons(); | |
| setInterval(hackathonAPISyncer.syncHackathons, 90 * 60 * 1000); | |
| // Update contests data and sync contests data at regular intervals | |
| await hackathonDBSyncer.updateHackathons(); | |
| setInterval(hackathonDBSyncer.updateHackathons, 60 * 60 * 1000); | |
| // Set up hackathon routes | |
| app.use("/hackathons", hackathonRoutes); | |
| } | |
| await dataSyncer.syncContests(); | |
| setInterval(async () => { | |
| try { | |
| await dataSyncer.syncContests(); | |
| } catch (err) { | |
| console.error("Contest sync failed:", err); | |
| } | |
| }, 90 * 60 * 1000); | |
| // Update contests data and sync contests data at regular intervals | |
| await contestSyncer.updateContests(); | |
| setInterval(async () => { | |
| try { | |
| await contestSyncer.updateContests(); | |
| } catch (err) { | |
| console.error("Contest update failed:", err); | |
| } | |
| }, 60 * 60 * 1000); | |
| // Pinging the server every 13 minutes | |
| setInterval( | |
| async () => { | |
| try { | |
| await main(); | |
| console.log("<=======Sent GET request to AWAKE"); | |
| } catch (error) { | |
| console.error("Error Pinging", error); | |
| } | |
| }, | |
| 13 * 60 * 1000, | |
| ); | |
| // Set up contest routes | |
| app.use("/contests", contestRoutes); | |
| } | |
| async function setupCommunityServer() { | |
| app.use("/community", communityRoutes); | |
| } | |
| async function setupHackathonServer() { | |
| await hackathonAPISyncer.syncHackathons(); | |
| setInterval(async () => { | |
| try { | |
| await hackathonAPISyncer.syncHackathons(); | |
| } catch (err) { | |
| console.error("Hackathon API sync failed:", err); | |
| } | |
| }, 90 * 60 * 1000); | |
| // Update contests data and sync contests data at regular intervals | |
| await hackathonDBSyncer.updateHackathons(); | |
| setInterval(async () => { | |
| try { | |
| await hackathonDBSyncer.updateHackathons(); | |
| } catch (err) { | |
| console.error("Hackathon DB update failed:", err); | |
| } | |
| }, 60 * 60 * 1000); | |
| // Set up hackathon routes | |
| app.use("/hackathons", hackathonRoutes); | |
| } |
🤖 Prompt for AI Agents
In backend/chore: Update index.js for stability and error logging around lines
97 to 135, several setInterval calls pass async functions directly which drops
returned promises and can turn transient rejections into unhandled promise
rejections; replace each setInterval(fnRef, ms) that currently uses an async
function reference (dataSyncer.syncContests, contestSyncer.updateContests,
hackathonAPISyncer.syncHackathons, hackathonDBSyncer.updateHackathons) with a
wrapper arrow that invokes the async call inside a try/catch and logs errors
(e.g., setInterval(async () => { try { await obj.method(); } catch (err) {
console.error("... failed", err); } }, ms)), ensuring the intervals still run at
the same intervals and adding clear error logging so failures don’t crash the
process.
Comment on lines
+239
to
+247
| // Handling unhandled server errors (for Unhandled Promise Rejections) | ||
| process.on("unhandledRejection", (err) => { | ||
| console.log(`Error: ${err.message}`); | ||
| console.log("Shutting down the server due to Unhandled promise rejection"); | ||
|
|
||
| appServer.close(() => { | ||
| process.exit(1); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard against undefined appServer in shutdown handler.
If an unhandled rejection fires before app.listen succeeds, appServer is still undefined and calling appServer.close throws, undermining the shutdown path. Check that appServer exists before closing.
-process.on("unhandledRejection", (err) => {
+process.on("unhandledRejection", (err) => {
console.log(`Error: ${err.message}`);
console.log("Shutting down the server due to Unhandled promise rejection");
-
- appServer.close(() => {
- process.exit(1);
- });
+ if (appServer && typeof appServer.close === "function") {
+ appServer.close(() => process.exit(1));
+ } else {
+ process.exit(1);
+ }
});📝 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
| // Handling unhandled server errors (for Unhandled Promise Rejections) | |
| process.on("unhandledRejection", (err) => { | |
| console.log(`Error: ${err.message}`); | |
| console.log("Shutting down the server due to Unhandled promise rejection"); | |
| appServer.close(() => { | |
| process.exit(1); | |
| }); | |
| }); | |
| // Handling unhandled server errors (for Unhandled Promise Rejections) | |
| process.on("unhandledRejection", (err) => { | |
| console.log(`Error: ${err.message}`); | |
| console.log("Shutting down the server due to Unhandled promise rejection"); | |
| if (appServer && typeof appServer.close === "function") { | |
| appServer.close(() => process.exit(1)); | |
| } else { | |
| process.exit(1); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In backend/chore: Update index.js for stability and error logging around lines
239 to 247, the unhandledRejection handler assumes appServer is defined and
calls appServer.close(), which will throw if app.listen hasn't set appServer
yet; update the handler to check if appServer is truthy before calling close,
and if not, log that no server was running and call process.exit(1) (or set a
safe fallback), ensuring graceful shutdown only attempts close when appServer
exists and always exits afterwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…d error logging
Pull Request Details
Description
[Provide a brief overview of the changes introduced by this pull request.]
Fixes
[Cite any related issues or bugs that this PR addresses, e.g., "Fixes #issueNumber"]
Type of PR
Summary
[Summarize the changes made in this PR.]
Screenshots (if applicable)
[If your changes include UI updates, provide screenshots to illustrate the changes.]
Additional Notes
[Include any additional information or context that might be helpful for reviewers.]
Checklist
npm run lint:fixandnpm run format:fix.Summary by CodeRabbit
Reliability Improvements
Performance
Monitoring
Chores