timeouts reworking #1531
Merged
timeouts reworking #1531
Conversation
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
…ling - Introduced a Timeout decorator to set custom timeout values for controllers. - Implemented TimeoutInterceptor to manage request timeouts based on the decorator. - Applied the Timeout decorator to various controllers: Dashboard Widgets, Dashboards, Saved DB Queries, Table Widget, and SaaS Controller. - Updated the TimeoutInterceptor to utilize the new decorator for dynamic timeout management. - Added default timeout values for production and testing environments.
There was a problem hiding this comment.
Pull request overview
This PR refactors the timeout mechanism from a global interceptor to a decorator-based approach, allowing for flexible, per-controller and per-endpoint timeout configurations.
Changes:
- Introduced a new
@Timeout()decorator with predefined timeout constants (DEFAULT, EXTENDED, AI) - Updated
TimeoutInterceptorto use NestJS Reflector to read timeout metadata from decorators - Removed the global
TimeoutInterceptorfromAPP_INTERCEPTORproviders inapp.module.ts - Applied
@Timeout()decorator to 25+ controllers at the class level - Applied method-level
@Timeout()decorators with extended values to specific endpoints (table operations, AI requests)
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/src/decorators/timeout.decorator.ts | New decorator definition with timeout constants |
| backend/src/interceptors/timeout.interceptor.ts | Updated to read metadata via Reflector |
| backend/src/app.module.ts | Removed global interceptor registration |
| backend/src/decorators/index.ts | Exported new decorator |
| backend/src/app.controller.ts | Added @timeout() decorator |
| backend/src/entities/*/**.controller.ts | Applied @timeout() to 24 entity controllers |
| backend/src/microservices/saas-microservice/saas.controller.ts | Applied @timeout() decorator |
| backend/src/entities/ai/user-ai-requests-v2.controller.ts | Added method-level @timeout with AI-specific values |
| backend/src/entities/table/table.controller.ts | Added method-level @timeout for extended operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const TimeoutDefaults = { | ||
| DEFAULT: 15000, | ||
| DEFAULT_TEST: 200000, | ||
| EXTENDED: 60000, |
There was a problem hiding this comment.
The EXTENDED_TEST timeout value is too high (300000ms = 5 minutes) compared to the EXTENDED value (60000ms = 1 minute). This is inconsistent with the DEFAULT/DEFAULT_TEST ratio where TEST is ~13x the normal value. Consider adjusting EXTENDED_TEST to match the pattern, or if this is intentional, add a comment explaining why.
Suggested change
| EXTENDED: 60000, | |
| EXTENDED: 60000, | |
| // Note: EXTENDED_TEST is intentionally capped at 5 minutes (5x EXTENDED) to avoid excessively long test runs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.