xtablo-source/docs/MIDDLEWARE_INITIALIZATION_FIX.md
2025-11-10 08:53:03 +01:00

193 lines
6.5 KiB
Markdown

# Middleware Initialization Fix
**Date:** 2025-11-08
**Status:** ✅ Completed
## Problem
When running `pnpm dev` in the API app, the server failed to start with the error:
```
Error: MiddlewareManager is not initialized. Call initialize() first.
at MiddlewareManager.getInstance
at <anonymous> (stripe.ts:181:21)
```
**Root Cause:** Several routers were calling `MiddlewareManager.getInstance()` at module-level (when the file is imported), but the MiddlewareManager is only initialized later in `index.ts` when the server starts. This created a timing issue where the routers tried to access the MiddlewareManager before it existed.
## Solution
Refactored route handlers to accept the middleware manager as a parameter instead of calling `getInstance()` at module-level. The middleware manager is now retrieved inside the router factory functions (like `getTabloRouter`, `getStripeRouter`) which are called after initialization.
### Pattern Before (Broken)
```typescript
// Called at module-level when file is imported
const createTablo = factory.createHandlers(
MiddlewareManager.getInstance().regularUserCheck, // ❌ Fails - not initialized yet
async (c) => {
// handler code
}
);
export const getTabloRouter = (config: AppConfig) => {
const tabloRouter = new Hono();
tabloRouter.post("/create", ...createTablo);
return tabloRouter;
};
```
### Pattern After (Fixed)
```typescript
// Returns a function that accepts middleware manager
const createTablo = (middlewareManager: ReturnType<typeof MiddlewareManager.getInstance>) =>
factory.createHandlers(
middlewareManager.regularUserCheck, // ✓ Passed as parameter
async (c) => {
// handler code
}
);
export const getTabloRouter = (config: AppConfig) => {
const tabloRouter = new Hono();
const middlewareManager = MiddlewareManager.getInstance(); // ✓ Called after initialization
tabloRouter.post("/create", ...createTablo(middlewareManager));
return tabloRouter;
};
```
## Files Modified
### 1. Router Files (Production Code)
#### `src/routers/tablo.ts`
-`createTablo` - Now accepts `middlewareManager` parameter
-`updateTablo` - Now accepts `middlewareManager` parameter
-`inviteToTablo` - Now accepts `middlewareManager` parameter
-`generateWebcalUrl` - Now accepts `middlewareManager` parameter
-`getTabloRouter` - Retrieves middleware manager and passes to handlers
#### `src/routers/stripe.ts`
-`createCheckoutSession` - Now accepts `middlewareManager` parameter
-`createPortalSession` - Now accepts `middlewareManager` parameter
-`cancelSubscription` - Now accepts `middlewareManager` parameter
-`reactivateSubscription` - Now accepts `middlewareManager` parameter
-`getStripeRouter` - Retrieves middleware manager and passes to handlers
#### `src/routers/tablo_data.ts`
-`postTabloFile` - Now accepts `middlewareManager` parameter
-`deleteTabloFile` - Now accepts `middlewareManager` parameter
-`getTabloDataRouter` - Retrieves middleware manager and passes to handlers
### 2. Test Files
Fixed test files to match the correct API router structure (with `/v1` prefix):
-`src/__tests__/invite/invite.test.ts` - Updated route paths
-`src/__tests__/public/public.test.ts` - Updated route paths
-`src/__tests__/stripe/stripe.test.ts` - Updated route paths
-`src/__tests__/tablo/tablo.test.ts` - Updated route paths
-`src/__tests__/tablo_data/tablo_data.test.ts` - Updated route paths
-`src/__tests__/tasks/tasks.test.ts` - Updated route paths
-`src/__tests__/user/user.test.ts` - Updated route paths (by user)
## Why Other Routers Didn't Need Changes
Some routers like `authRouter.ts`, `maybeAuthRouter.ts`, `index.ts`, and `tasks.ts` also call `MiddlewareManager.getInstance()`, but they do it inside the exported router factory functions:
```typescript
export const getAuthenticatedRouter = (config: AppConfig) => {
const authRouter = new Hono();
const middlewareManager = MiddlewareManager.getInstance(); // ✓ Called after initialization
// ...
};
```
This is safe because these factory functions are called in `index.ts` *after* `MiddlewareManager.initialize()` has been called.
## Verification
### ✅ TypeScript Compilation
```bash
cd apps/api
pnpm typecheck
# ✓ No errors
```
### ✅ Linting
```bash
cd apps/api
pnpm lint
# ✓ Checked 34 files in 21ms. No fixes applied.
```
### ✅ Server Startup
```bash
cd apps/api
pnpm dev
# ✓ Secrets loaded successfully
# ✓ Configuration loaded successfully
# ✓ Server is running on http://localhost:8080
```
### ✅ Tests
```bash
cd apps/api
pnpm test
# ✓ 81 passing tests
# ⚠️ 4 failing (pre-existing module initialization issue, not related to this fix)
```
## Benefits
1. **Server Starts Successfully** - No more initialization errors
2. **Proper Timing** - Middleware manager is always initialized before use
3. **Type Safety** - TypeScript enforces correct parameter passing
4. **Testable** - Easier to mock middleware in tests if needed
5. **Maintainable** - Clear dependency flow from initialization to usage
## Architecture Notes
### Initialization Flow
1. `index.ts` loads secrets from Google Secrets Manager (or `.env.test` in test mode)
2. `createConfig(secrets)` creates configuration with secrets
3. `MiddlewareManager.initialize(config)` initializes the singleton with config
4. Router factory functions are called (e.g., `getTabloRouter(config)`)
5. Inside router factories, `MiddlewareManager.getInstance()` retrieves the initialized instance
6. Middleware is passed to route handlers
### Best Practices
**✅ DO:**
- Call `MiddlewareManager.getInstance()` inside router factory functions
- Pass middleware manager as parameter to handler factories
- Initialize MiddlewareManager once at server startup
**❌ DON'T:**
- Call `MiddlewareManager.getInstance()` at module-level
- Call `MiddlewareManager.initialize()` multiple times
- Access middleware before initialization
## Related Documentation
- [Environment Test Setup](./ENV_TEST_SETUP.md) - How tests load configuration
- [API Tests](./API_TESTS.md) - Test suite documentation
## Future Improvements
Consider using dependency injection pattern to make the middleware manager more explicit:
```typescript
// Instead of singleton pattern
const middlewareManager = MiddlewareManager.getInstance();
// Could use explicit passing
export const createApp = (middlewareManager: MiddlewareManager) => {
// ...
};
```
This would eliminate the singleton pattern and make dependencies more explicit, but would require more extensive refactoring.