Skip to content

add error handling and graceful shutdown #302

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
60 changes: 59 additions & 1 deletion apps/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Connect, Identity, Inboxes, Messages, SpaceEvents, Utils } from '@graph
import { bytesToHex, randomBytes } from '@noble/hashes/utils.js';
import cors from 'cors';
import { Effect, Exit, Schema } from 'effect';
import express, { type Request, type Response } from 'express';
import express, { type NextFunction, type Request, type Response } from 'express';
import WebSocket, { WebSocketServer } from 'ws';
import { addAppIdentityToSpaces } from './handlers/add-app-identity-to-spaces.js';
import { applySpaceEvent } from './handlers/applySpaceEvent.js';
Expand Down Expand Up @@ -68,6 +68,14 @@ app.use(express.json({ limit: '2mb' }));

app.use(cors());

// Request timeout middleware
app.use((req: Request, res: Response, next: NextFunction) => {
res.setTimeout(30000, () => {
res.status(408).json({ error: 'Request timeout' });
});
Comment on lines +73 to +75
Copy link
Preview

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

After sending the timeout response, ensure no further handlers write to the response (e.g., by storing a flag or directly ending the response) to avoid double send errors.

Suggested change
res.setTimeout(30000, () => {
res.status(408).json({ error: 'Request timeout' });
});
let responseSent = false;
res.setTimeout(30000, () => {
if (!responseSent) {
responseSent = true;
res.status(408).json({ error: 'Request timeout' });
}
});
res.on('finish', () => {
responseSent = true;
});

Copilot uses AI. Check for mistakes.

next();
});

Copy link
Preview

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

The error and 404 handlers are registered before your routes, so they’ll intercept all requests and prevent your GET '/' and other routes from ever being reached. Move these middleware calls to the end—after you define all route handlers.

Suggested change
app.get('/', (_req, res) => {
res.send('Server is running (v0.0.10)');
});

Copilot uses AI. Check for mistakes.

app.get('/', (_req, res) => {
res.send('Server is running (v0.0.10)');
});
Expand Down Expand Up @@ -553,10 +561,60 @@ app.post('/accounts/:accountAddress/inboxes/:inboxId/messages', async (req, res)
broadcastAccountInboxMessage({ accountAddress, inboxId, message: createdMessage });
});

// Global error handling middleware
app.use((error: Error, req: Request, res: Response, next: NextFunction) => {
console.error('Unhandled error:', error);
res.status(500).json({
error: 'Internal server error',
message: process.env.NODE_ENV === 'development' ? error.message : 'Something went wrong',
});
});

// 404 handler
app.use('*', (req: Request, res: Response) => {
res.status(404).json({ error: 'Route not found' });
});

const server = app.listen(PORT, () => {
console.log(`Listening on port ${PORT}`);
});

// Global process error handlers
process.on('unhandledRejection', (reason, promise) => {
console.error('Unhandled Rejection at:', promise, 'reason:', reason);
Copy link
Preview

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

Uncaught promise rejections can leave the process in an inconsistent state. Consider invoking the same shutdown logic as for uncaughtException or at least logging and exiting after a grace period.

Suggested change
console.error('Unhandled Rejection at:', promise, 'reason:', reason);
console.error('Unhandled Rejection at:', promise, 'reason:', reason);
// Graceful shutdown
server.close(() => {
console.log('Server closed due to unhandled rejection');
webSocketServer.close(() => {
console.log('WebSocket server closed');
process.exit(1);
});
});
// Force close after 30 seconds
setTimeout(() => {
console.error('Could not close connections in time, forcefully shutting down');
process.exit(1);
}, 30000);

Copilot uses AI. Check for mistakes.

});

process.on('uncaughtException', (error) => {
Copy link
Preview

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

In this handler, you close only the HTTP server. To fully shut down, also call webSocketServer.close() before exiting to avoid leaving socket connections open.

Copilot uses AI. Check for mistakes.

console.error('Uncaught Exception:', error);
// Graceful shutdown
server.close(() => {
console.log('Server closed due to uncaught exception');
process.exit(1);
});
});

// Graceful shutdown handlers
const gracefulShutdown = (signal: string) => {
console.log(`Received ${signal}. Starting graceful shutdown...`);

server.close(() => {
console.log('HTTP server closed');
webSocketServer.close(() => {
console.log('WebSocket server closed');
process.exit(0);
});
});

// Force close after 30 seconds
setTimeout(() => {
console.error('Could not close connections in time, forcefully shutting down');
process.exit(1);
}, 30000);
};

process.on('SIGTERM', () => gracefulShutdown('SIGTERM'));
process.on('SIGINT', () => gracefulShutdown('SIGINT'));

function broadcastSpaceEvents({
spaceId,
event,
Expand Down
Loading