feat(apps/server/server-rest): logging & cors#102
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances observability by integrating a centralized console-logger, adds caching/rate-limit guards for external APIs, and tightens CORS settings.
- Introduced
LogMetadatadecorators on controllers and a globalLoggingInterceptorfor structured request logging. - Implemented
ApiCacheServiceandGithubApiServiceto cache WakaTime/GitHub data and respect API rate limits. - Updated
main.tsto configure console-logger and extend CORS origins.
Reviewed Changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/rest/src/main.ts | Configures console-logger and extends CORS rules |
| apps/server/rest/src/common/common.module.ts | Registers global logging interceptor and API module |
| apps/server/rest/src/common/services/api-cache.service.ts | Implements cache wrapper with delay logic |
| apps/server/rest/src/common/services/github-api.service.ts | Wraps GitHub GraphQL/REST calls with rate checks |
| apps/server/rest/src/common/interceptors/logging.interceptor.ts | Adds basic request/response timing logs |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/server/rest/src/common/common.module.ts:26
- Exporting
ApiModuleinstead of its providers may confuse consumers. Consider exporting the services (ApiCacheService,GithubApiService) directly or re-exporting the module inapp.module.ts.
exports: [ServerlessProxyService, ApiModule],
| @@ -0,0 +1,28 @@ | |||
| import { applyDecorators } from '@nestjs/common'; | |||
There was a problem hiding this comment.
The LogRoute decorator is imported in health.controller.ts but never applied to any method. Consider removing the unused decorator or applying it where needed.
| @@ -0,0 +1,59 @@ | |||
| import { Observable } from 'rxjs'; | |||
There was a problem hiding this comment.
The AdvancedLoggingInterceptor is implemented but not registered in CommonModule. Register it via APP_INTERCEPTOR or remove it if unused.
|
|
||
| import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common'; | ||
|
|
||
| import { log } from '@99mini/console-logger'; |
There was a problem hiding this comment.
[nitpick] The interceptor uses log(...) but elsewhere code uses info(...)/error(...). Align on one log level API for consistency.
| import { log } from '@99mini/console-logger'; | |
| import { info } from '@99mini/console-logger'; |
No description provided.