-
Notifications
You must be signed in to change notification settings - Fork 574
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
Fix/server app api not running #8609
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces significant modifications across multiple files in the Gauzy application, focusing on enhancing configuration, error handling, and plugin integration. Key changes include adding a Changes
Sequence DiagramsequenceDiagram
participant Client
participant ElectronApp
participant ServerAPI
participant Plugins
Client->>ElectronApp: Trigger App Update Check
ElectronApp->>ServerAPI: Request Update Information
ServerAPI->>Plugins: Fetch Plugin Configurations
Plugins-->>ServerAPI: Return Plugin Details
ServerAPI-->>ElectronApp: Send Update Information
ElectronApp->>Client: Display Update Options
Possibly Related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a typosquat?Package name is similar to other popular packages and may not be the package you want. Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages. What are http dependencies?Contains a dependency which resolves to a remote HTTP URL which could be used to inject untrusted code and reduce overall package reliability. Publish the HTTP URL dependency to npm or a private package repository and consume it from there. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/server-api/src/index.ts (1)
338-338
: Add a log statement or comment explaining the purpose of the DEBUG flag.
It’s useful to explain how the DEBUG variable interacts with the rest of the system. Otherwise, future maintainers might remove or overlook it if they do not recognize its significance.apps/server/src/package.json (3)
31-35
: Consider consistent ordering of workspace paths.
Keeping workspace paths alphabetically or logically grouped can improve maintainability.
137-149
: Evaluate plugin ordering or loading priority.
If these plugins rely on a specific load order, you may need a note or commented reference to clarify that some plugins’ functionalities are interdependent.
185-337
: Assess performance impact of newly added @opentelemetry dependencies.
Tracing and instrumentation can add overhead. Validate that high-traffic endpoints are still performant with OpenTelemetry instrumentation enabled.apps/server-api/src/package.json (1)
186-206
: Enhance observability with OpenTelemetry configurationThe addition of OpenTelemetry packages provides powerful observability capabilities. Consider implementing proper configuration to help diagnose API issues:
- Set up trace sampling and exporters
- Configure metrics collection
- Implement proper context propagation
Would you like me to provide a configuration example for setting up OpenTelemetry?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/server-api/src/index.ts
(1 hunks)apps/server-api/src/package.json
(3 hunks)apps/server/src/index.ts
(1 hunks)apps/server/src/package.json
(3 hunks)package.json
(1 hunks)packages/core/src/lib/core/core.module.ts
(1 hunks)packages/desktop-lib/src/lib/server/service/server-task.ts
(1 hunks)
🔇 Additional comments (9)
packages/core/src/lib/core/core.module.ts (1)
33-33
: Update documentation or comments with rationale for the path change.
The new path references an additional directory level (“../../../../../../../data/”). It would be helpful to document why this is necessary or to confirm that the path is correct to avoid confusion.
✅ Verification successful
Let me gather more context about the build process and file structure.
Let me check the context around this path resolution in the core module to understand its purpose.
Path change is justified by build process and environment differences
The path difference is actually correct and justified by the following findings:
- For Electron (desktop) builds,
.gql
files are copied todist/apps/desktop/data/
as shown in package.json'scopy-files-desktop
script - For non-Electron environments, it uses the standard path to schema files within the source tree
- This is properly handled by the environment condition
environment.isElectron
The path with multiple parent directories (../../../../../../../data/
) is necessary to reach the copied .gql
files in the Electron build's directory structure, while the alternative path (../**/schema/*.gql
) is used for server environments where files remain in their original location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# This script checks for references to .gql files in the codebase,
# ensuring that the updated path is correctly used.
rg "\.gql"
Length of output: 588
Script:
#!/bin/bash
# Check the directory structure and build scripts
fd -t f -e gql
Length of output: 111
Script:
#!/bin/bash
# Get the context of the path resolution in core.module.ts
rg -B 5 -A 5 "\.gql" packages/core/src/lib/core/core.module.ts
Length of output: 556
packages/desktop-lib/src/lib/server/service/server-task.ts (1)
62-62
: Validate if the 2-minute timeout still meets user experience requirements.
Prolonging the timeout from 1 minute to 2 minutes can keep the user waiting longer if something goes wrong. Make sure this is intentional and beneficial, especially if error-handling is time-sensitive.
✅ Verification successful
Let me gather more context about the timeout usage and error handling.
Let me gather more context about the server task implementation and timeout handling.
Let me check for any error handling or critical message patterns that might be related to timeouts.
Based on the gathered context, I can now provide a final response about the timeout change:
2-minute timeout is appropriate for server task initialization
The 2-minute timeout on runTask
is reasonable considering:
- It's used for server initialization which may require more time than regular operations
- The class has proper error handling with critical error detection (
criticalMessageError
) and logging - Failed starts are properly handled through the error handler which kills the process and notifies observers
- Users are kept informed through logger notifications and state updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for references to runTask calls that rely on a specific timeout logic.
rg -A 5 "runTask"
Length of output: 1134
Script:
#!/bin/bash
# Check the full implementation of the ServerTask class to understand the timeout and error handling
ast-grep --pattern 'class ServerTask {
$$$
}'
# Also check for any error handling related to timeouts
rg -B 2 -A 5 "handleError|onTimeout"
Length of output: 65948
Script:
#!/bin/bash
# Check the full implementation of the ServerTask class and its timeout handling
rg -B 5 -A 10 "@Timeout" packages/desktop-lib/src/lib/server/service/server-task.ts
# Also check for any timeout-related configuration or constants
rg "timeout|TIMEOUT" packages/desktop-lib/src/lib/server/service/server-task.ts
Length of output: 704
Script:
#!/bin/bash
# Check for critical message patterns and error handling in ServerTask
rg -B 10 -A 5 "criticalMessageError|errorMessage" packages/desktop-lib/src/lib/server/service/server-task.ts
# Also check the full class implementation to understand the context
ast-grep --pattern 'class ServerTask {
$$$
}'
Length of output: 2257
apps/server/src/package.json (4)
39-41
: Double-check that “--production” is appropriate for all build scenarios.
This ensures no dev dependencies are installed, but confirm that dev scripts or functionalities are not needed during packaging.
135-135
: Ensure “@gauzy/auth” is still in use.
Confirm presence of references to “@gauzy/auth” in code. If it is no longer used, remove it to minimize dependencies.
✅ Verification successful
"@gauzy/auth" package is actively used in the codebase
The package is imported and utilized in core modules, specifically in auth-related functionality:
- Used in
packages/core/src/lib/auth/auth.module.ts
(importsSocialAuthModule
) - Used in
packages/core/src/lib/auth/auth.service.ts
(importsSocialAuthService
) - Required as a dependency in multiple package.json files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for import references to @gauzy/auth
rg "@gauzy/auth"
Length of output: 793
150-152
: Confirm "@gauzy/desktop-lib" and "@gauzy/desktop-window" version alignment across repo.
Version mismatches can introduce subtle runtime errors.
✅ Verification successful
Version alignment check reveals mixed dependency references
The verification shows inconsistent version references across the repository:
-
Some packages use fixed version
^0.1.0
:- apps/server/src/package.json
- apps/server-api/src/package.json
- packages/desktop-lib/package.json (for desktop-window)
- packages/desktop-ui-lib/package.json (for desktop-window)
-
Others use local file references:
- apps/desktop/src/package.json
- apps/desktop-timer/src/package.json
This mixed usage of versioned dependencies and local file references is intentional for development purposes, where desktop apps use local builds while server components use fixed versions. No action needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to desktop-lib and desktop-window versions in package.json files across the repo
fd package.json | xargs rg '"@gauzy/desktop-lib"'
fd package.json | xargs rg '"@gauzy/desktop-window"'
Length of output: 1089
25-26
: Verify new plugin-based workspace paths.
These lines add references to new plugin packages in the workspace. Confirm that these plugins exist and build successfully.
✅ Verification successful
Both plugins exist and are properly structured in the codebase
The verification confirms that both plugins are present and properly structured:
integration-ai
plugin exists with complete implementation including controllers, services, and modulesjob-proposal
plugin exists with proper plugin implementation and UI components
Both plugins have their source files in the correct locations under packages/plugins/
and have proper package.json configurations with all necessary dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for the existence of these plugin directories.
fd "integration-ai" | grep "dist/packages/plugins/integration-ai"
fd "job-proposal" | grep "dist/packages/plugins/job-proposal"
Length of output: 123
Script:
#!/bin/bash
# Let's check for the existence of these plugins in the source code
fd "integration-ai|job-proposal" packages/plugins
# Let's also check package.json files in the plugins directory
fd "package.json" packages/plugins -x cat {}
Length of output: 43656
apps/server-api/src/package.json (2)
207-209
: Configure Apollo Server integration
The addition of Apollo packages suggests GraphQL API implementation. Ensure proper setup:
- Configure schema and resolvers
- Set up proper error handling
- Implement authentication/authorization
✅ Verification successful
Let me generate additional verification scripts to check for resolver implementations and GraphQL setup:
The previous results showed GraphQL schema files but we need to verify the actual server setup and resolver implementations to ensure proper configuration. These additional checks will help determine if the Apollo Server integration is properly configured.
GraphQL setup is properly configured
The codebase shows a complete GraphQL setup with Apollo Server:
- GraphQL module is configured in
packages/core/src/lib/graphql/graphql.module.ts
using@nestjs/apollo
- Schema definitions exist in multiple locations including role management
- Apollo Server plugins and configuration are properly set up in multiple config files
- Integration with NestJS is handled via the Apollo Driver
The Apollo packages added in package.json align with the existing GraphQL implementation. No additional setup is required as the framework is already properly integrated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for GraphQL schema definitions
find . -type f -name "*.gql" -o -name "*.graphql" | while read -r file; do
echo "Found GraphQL schema: $file"
cat "$file"
done
# Check for resolver implementations
ast-grep --pattern 'class $_ {
$$$
@Resolver($_)
$$$
}'
Length of output: 70148
Script:
#!/bin/bash
# Search for GraphQL resolver implementations
ast-grep --pattern 'class $_ {
$$$
@Resolver()
$$$
}'
# Search for GraphQL module configuration
rg -l "ApolloServer|GraphQLModule" --type ts
# Check for GraphQL middleware/setup code
rg -A 5 "apollo" --type ts
Length of output: 4492
25-31
: Verify plugin workspace dependencies
The addition of new plugin workspaces (integration-ai
, job-proposal
, ui-config
) suggests expanded functionality. Ensure these plugins are properly initialized in the server startup sequence.
package.json (1)
267-267
:
Verify build dependencies after script simplification
The build:gauzy-server
script has been simplified by removing build:package:all
. This could lead to missing dependencies if packages aren't built in the correct order.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
View your CI Pipeline Execution ↗ for commit 43c77a0.
☁️ Nx Cloud last updated this comment at |
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation