fix: 🐛 serve error of users app #93
Conversation
WalkthroughThe changes in this pull request involve the integration of an Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
libs/auth/application/src/lib/auth.service.ts (2)
Line range hint
16-48: Implement structured error handling with error codesThe current implementation has multiple TODO comments about error codes and uses string messages. Consider implementing a proper error handling structure.
+enum AuthErrorCode { + INVALID_CREDENTIALS = 'AUTH001', + USER_NOT_FOUND = 'AUTH002', + COGNITO_ERROR = 'AUTH003' +} + +interface AuthError { + code: AuthErrorCode; + message: string; +} + async signIn( email: string, pass: string, ): Promise<{ accessToken: string; }> { try { await this.awsCognitoService.signIn(email, pass); } catch (error) { this.logger.error('SignIn error:', error); - throw new UnauthorizedException(error); // TODO: return error code + throw new UnauthorizedException({ + code: AuthErrorCode.COGNITO_ERROR, + message: error.message + }); } try { const user = await this.usersService.findByEmail(email); if (!user) { - throw 'User is not found after validated user credentials'; // TODO: return error code + throw new UnauthorizedException({ + code: AuthErrorCode.USER_NOT_FOUND, + message: 'User not found after validation' + }); } // ... rest of the code } catch (error) { - throw new UnauthorizedException('Invalid credentials'); // TODO: return error code + throw new UnauthorizedException({ + code: AuthErrorCode.INVALID_CREDENTIALS, + message: 'Invalid credentials' + }); } }
Line range hint
16-48: Add input validation for email and passwordThe method lacks input validation for email and password parameters. Consider adding validation to ensure data integrity.
+import { IsEmail, MinLength } from 'class-validator'; + +class SignInDto { + @IsEmail() + email: string; + + @MinLength(8) + password: string; +} + async signIn( email: string, pass: string, ): Promise<{ accessToken: string; }> { + // Validate input + const signInDto = new SignInDto(); + signInDto.email = email; + signInDto.password = pass; + await validate(signInDto); + try { await this.awsCognitoService.signIn(email, pass); // ... rest of the codelibs/auth/interface-adapters/src/lib/auth.module.ts (1)
23-26: Consider Removing Redundant ProviderJwtServiceSince
JwtModuleis globally registered usingregisterAsync(), theJwtServiceis already available for injection throughout the application. IncludingJwtServiceexplicitly in theprovidersarray may be unnecessary.Apply this diff to remove the redundant provider:
providers: [ AuthResolver, AuthService, AwsCognitoService, UsersService, - JwtService, GetUserUseCase, { provide: USERS_REPOSITORY, useClass: MongooseUsersRepository, }, ],libs/auth/interface-adapters/src/lib/resolver/auth.resolver.ts (1)
6-10: Rename ClassResponseto Avoid Naming ConflictsThe class name
Responseis very generic and may conflict with other classes or interfaces namedResponse, especially in HTTP contexts. Consider renaming it to a more specific name likeSignInResponseto improve clarity and prevent potential conflicts.Apply this diff to rename the class:
@ObjectType() -export class Response { +export class SignInResponse { @Field() accessToken!: string; } @Resolver() export class AuthResolver { // ... - @Mutation(() => Response) + @Mutation(() => SignInResponse) async signIn( @Args({ name: 'signInInput', type: () => SignInInputDto }) signInInput: SignInInputDto, ): Promise<SignInResponse> { // ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
apps/users/src/app/app.module.ts(2 hunks)libs/auth/application/src/lib/auth.service.ts(1 hunks)libs/auth/interface-adapters/src/lib/auth.module.ts(1 hunks)libs/auth/interface-adapters/src/lib/dto/sign-in-input.dto.ts(1 hunks)libs/auth/interface-adapters/src/lib/resolver/auth.resolver.ts(1 hunks)libs/users/interface-adapters/src/lib/dto/user.dto.ts(1 hunks)
🔇 Additional comments (7)
libs/auth/application/src/lib/auth.service.ts (2)
1-9: LGTM! Follows NestJS best practices
The class is properly decorated with @Injectable() and follows the dependency injection pattern. Logger initialization as a class property is a clean approach.
10-14: Verify potential circular dependencies
The service has multiple dependencies. Let's ensure there are no circular dependency issues between these services.
✅ Verification successful
Based on the comprehensive search results, I can now generate the final response:
No circular dependencies detected
The dependency analysis shows a clean, unidirectional dependency flow:
- AuthService depends on UsersService, but UsersService doesn't import anything from the auth module
- AwsCognitoService and JwtService are utility services with no dependencies on AuthService
- UsersService has minimal dependencies (only GetUserUseCase) and doesn't import from auth module
The service architecture follows proper dependency injection patterns without creating any circular references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential circular dependencies between services
# Search for imports of AuthService in the dependent services
echo "Checking for potential circular dependencies..."
rg -l "import.*AuthService" --type ts
# Check if UsersService imports anything from auth module
rg -l "import.*auth" --type ts | grep -i "users.service"
Length of output: 435
Script:
#!/bin/bash
# Let's check for potential circular dependencies more thoroughly
# 1. Check if UsersService imports anything from auth module with broader pattern
echo "Checking UsersService imports from auth module..."
rg "from.*auth" -C 2 --type ts | grep -i "users.service"
# 2. Check the implementation of UsersService to see its dependencies
echo -e "\nChecking UsersService implementation..."
ast-grep --pattern 'class UsersService {
constructor($_) {
$$$
}
}'
# 3. Check if AwsCognitoService or JwtService import anything from auth module
echo -e "\nChecking AwsCognitoService and JwtService imports..."
rg "from.*auth" -C 2 --type ts | grep -iE "(awscognito|jwt).service"
# 4. Look for their implementations to verify dependencies
echo -e "\nChecking AwsCognitoService and JwtService implementations..."
ast-grep --pattern 'class $_ {
constructor($_) {
$$$
}
}'
Length of output: 735
Script:
#!/bin/bash
# Let's try a different approach to find any circular dependencies
# 1. First, let's find all service files that might be relevant
echo "Finding relevant service files..."
fd -e ts -E "*.spec.ts" -E "*.test.ts" service
# 2. For each found service, check its content for imports and class structure
echo -e "\nChecking imports and class structure in UsersService..."
rg "import.*from|@Injectable|class.*Service" -C 2 $(fd -e ts -E "*.spec.ts" -E "*.test.ts" "users.service")
echo -e "\nChecking imports and class structure in AwsCognitoService..."
rg "import.*from|@Injectable|class.*Service" -C 2 $(fd -e ts -E "*.spec.ts" -E "*.test.ts" "aws-cognito.service")
echo -e "\nChecking imports and class structure in JwtService..."
rg "import.*from|@Injectable|class.*Service" -C 2 $(fd -e ts -E "*.spec.ts" -E "*.test.ts" "jwt.service")
echo -e "\nChecking imports and class structure in AuthService..."
rg "import.*from|@Injectable|class.*Service" -C 2 $(fd -e ts -E "*.spec.ts" -E "*.test.ts" "auth.service" -E "*.spec.ts")
Length of output: 29698
libs/auth/interface-adapters/src/lib/auth.module.ts (2)
39-42:
Confirm Configuration Key for JWT Secret
The useFactory function retrieves authConfig using configService.get('auth'). Verify that the auth configuration contains a secret property. If your JWT secret is stored under a different key, such as jwt, update the configuration retrieval accordingly.
Apply this diff if the configuration key should be jwt:
const authConfig = configService.get('auth');
return {
- secret: authConfig.secret,
+ secret: authConfig.jwtSecret,
signOptions: { expiresIn: '60s' },
};Run the following script to confirm the presence of the secret in the auth configuration:
✅ Verification successful
Configuration Key for JWT Secret is Correctly Used
The verification confirms that the auth configuration object correctly includes a secret property, which is populated from the environment variable JWT_SECRET. The current implementation in auth.module.ts using authConfig.secret is correct and aligned with the configuration structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if 'auth' configuration includes 'secret' property
rg "registerAs\('auth'" -A 5 -t ts
Length of output: 458
28-30: Verify the Injection Token for USERS_REPOSITORY
Ensure that the injection token USERS_REPOSITORY matches the token used in other parts of the application where the users repository is injected. Consistency in injection tokens is crucial for the dependency injection system to function correctly.
Run the following script to check for the usage of USERS_REPOSITORY:
✅ Verification successful
Let's search for both the usage of the token and its declaration to verify consistency.
Injection token USERS_REPOSITORY is consistently used across the codebase
The token is properly defined in libs/users/domain/src/lib/users.repository.ts as a Symbol and is consistently imported and used across multiple modules:
- Defined as
Symbol('USERS_REPOSITORY')in users domain - Used in users module for dependency injection
- Used in get-user use case with
@Injectdecorator - Used in auth module (the file under review)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of USERS_REPOSITORY injection token
rg "Inject\((['\"])USERS_REPOSITORY\1\)" -t ts
Length of output: 282
Script:
#!/bin/bash
# Search for USERS_REPOSITORY token usage
rg "USERS_REPOSITORY" -t ts
# Search for any injection token declarations
ast-grep --pattern 'export const USERS_REPOSITORY = $_;'
Length of output: 905
libs/auth/interface-adapters/src/lib/dto/sign-in-input.dto.ts (1)
3-3: Addition of @InputType() is Appropriate
The introduction of the @InputType() decorator aligns the SignInInputDto with GraphQL standards for input types. This change ensures that the DTO can be used correctly in GraphQL mutations.
apps/users/src/app/app.module.ts (1)
22-22: AuthModule Integration Looks Good
The addition of AuthModule to the imports array successfully integrates authentication features into the application. Ensure that the required configurations are correctly set up for seamless operation.
libs/auth/interface-adapters/src/lib/resolver/auth.resolver.ts (1)
21-21:
Ensure AuthService.signIn Returns Correct Structure
The signIn method should return an object matching the SignInResponse type, containing an accessToken field. Verify that AuthService.signIn returns the expected structure to prevent runtime errors.
Run the following script to check the return value of AuthService.signIn:
✅ Verification successful
Return Type of AuthService.signIn is Correctly Structured
The signIn method in AuthService correctly returns an object with an accessToken property of type string, which matches the expected structure. The implementation shows proper JWT token generation and return format:
return {
accessToken,
};🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that AuthService.signIn returns an object with 'accessToken' property
rg "signIn\(.*\).*{[^}]*return.*}" -A 10 -t ts @auth/application/src/lib/auth.service.ts
Length of output: 217
Script:
#!/bin/bash
# Let's try a simpler pattern to find the signIn method implementation
ast-grep --pattern 'signIn($_) {
$$$
}'
Length of output: 43
Script:
#!/bin/bash
# Search for SignInResponse type definition
rg "type SignInResponse" -A 5
Length of output: 31
Script:
#!/bin/bash
# Search for AuthService class implementation
rg "class AuthService" -A 20
Length of output: 1596
Script:
#!/bin/bash
# Let's see more of the signIn method implementation as it was cut off
rg "async signIn" -B 2 -A 30 libs/auth/application/src/lib/auth.service.ts
Length of output: 923
|
🎉 This PR is included in version 1.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Related: #71
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes