fix: Integration Chatwoot and Baileys services #2158
Merged
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
Contributor
Reviewer's GuideThis PR refines the Chatwoot-Baileys integration by enhancing logging and caching in ChatwootService, standardizing DTOs and controller logic, improving instance lifecycle management, consolidating phone formatting and message handling, cleaning up utilities, and updating versioning and dependencies. Sequence diagram for improved instance restart logic in InstanceControllersequenceDiagram
participant User
participant InstanceController
participant Instance
participant WAMonitoringService
User->>InstanceController: Request to restart instance
InstanceController->>Instance: Check connectionStatus
alt Instance has restart() method
InstanceController->>Instance: Call restart()
InstanceController->>Instance: Wait for reconnection
InstanceController-->>User: Return instance status
else Baileys fallback
InstanceController->>Instance: Close ws and end client
InstanceController->>WAMonitoringService: connectToWhatsapp()
WAMonitoringService-->>InstanceController: Instance reconnected
InstanceController-->>User: Return instance status
end
Sequence diagram for improved instance deletion timeout management in WAMonitoringServicesequenceDiagram
participant WAMonitoringService
participant Instance
participant EventEmitter
WAMonitoringService->>WAMonitoringService: delInstanceTime(instance)
alt Timeout exists
WAMonitoringService->>WAMonitoringService: clear previous timeout
end
WAMonitoringService->>WAMonitoringService: set new timeout
WAMonitoringService->>Instance: Check connectionStatus
alt connectionStatus == 'connecting' and integration == Baileys
Instance->>Instance: logout()
Instance->>Instance: ws.close()
Instance->>Instance: end()
WAMonitoringService->>EventEmitter: emit remove.instance
else
WAMonitoringService->>EventEmitter: emit remove.instance
end
WAMonitoringService->>WAMonitoringService: delete timeout reference
Class diagram for updated InstanceDto and related typesclassDiagram
class InstanceDto {
+instanceName: string
+instanceId: string
+connectionStatus: string
+token: string
+status: string
+ownerJid: string
+profileName: string
+profilePicUrl: string
...
}
class IntegrationDto {
...
}
InstanceDto --|> IntegrationDto
class wa.Instance {
+instanceName: string
+instanceId: string
+connectionStatus: string
+ownerJid: string
...
}
InstanceDto <.. wa.Instance : used for data transfer
Class diagram for updated ChatwootController and ChatwootService relationshipclassDiagram
class ChatwootController {
-chatwootService: ChatwootService
-configService: ConfigService
+createChatwoot(instance: InstanceDto, data: ChatwootDto)
+receiveWebhook(instance: InstanceDto, data: any)
}
class ChatwootService {
...
}
ChatwootController --> ChatwootService : uses
Class diagram for WAMonitoringService instance timeout managementclassDiagram
class WAMonitoringService {
+waInstances: Record<string, any>
-delInstanceTimeouts: Record<string, NodeJS.Timeout>
+delInstanceTime(instance: string)
+clearDelInstanceTime(instance: string)
...
}
Class diagram for updated phone number formatting in ChatwootServiceclassDiagram
class ChatwootService {
+parsePhoneNumberFromString(phone: string): PhoneNumber
...
}
class PhoneNumber {
+formatInternational(): string
...
}
ChatwootService --> PhoneNumber : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:615` </location>
<code_context>
const conversationId = (await this.cache.get(cacheKey)) as number;
this.logger.verbose(`Found conversation to: ${phoneNumber}, conversation ID: ${conversationId}`);
- let conversationExists: conversation | boolean;
+ let conversationExists: any;
try {
conversationExists = await client.conversations.get({
</code_context>
<issue_to_address>
**suggestion:** Switching conversationExists type to any reduces type safety.
Consider using a union type or a descriptive type alias if the value can be multiple types, rather than 'any', to maintain type safety and clarity.
Suggested implementation:
```typescript
type ConversationOrFalse = conversation | false;
let conversationExists: ConversationOrFalse;
```
- If the `conversation` type is not already imported or defined, ensure it is imported from the correct module.
- If you prefer, you can define the type alias at the top of the file or near other type definitions for better visibility.
</issue_to_address>
### Comment 2
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:622` </location>
<code_context>
});
- this.logger.verbose(`Conversation exists: ${JSON.stringify(conversationExists)}`);
+ this.logger.verbose(
+ `Conversation exists: ID: ${conversationExists.id} - Name: ${conversationExists.meta.sender.name} - Identifier: ${conversationExists.meta.sender.identifier}`,
+ );
} catch (error) {
</code_context>
<issue_to_address>
**issue:** Logging assumes meta.sender properties always exist.
If conversationExists or meta.sender may be undefined, use optional chaining or fallback values to prevent runtime errors in the log statement.
</issue_to_address>
### Comment 3
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:675` </location>
<code_context>
this.logger.verbose(`Processing group conversation`);
const group = await this.waMonitor.waInstances[instance.instanceName].client.groupMetadata(chatId);
- this.logger.verbose(`Group metadata: ${JSON.stringify(group)}`);
+ this.logger.verbose(`Group metadata: JID:${group.JID} - Subject:${group?.subject || group?.Name}`);
const participantJid = isLid && !body.key.fromMe ? body.key.participantAlt : body.key.participant;
</code_context>
<issue_to_address>
**nitpick (typo):** Potential typo: group.JID may be undefined.
Verify whether the correct property is 'id' instead of 'JID' to avoid logging undefined values.
```suggestion
this.logger.verbose(`Group metadata: ID:${group.id} - Subject:${group?.subject || group?.Name}`);
```
</issue_to_address>
### Comment 4
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:803-811` </location>
<code_context>
if (await this.cache.has(cacheKey)) {
const conversationId = (await this.cache.get(cacheKey)) as number;
this.logger.verbose(`Found conversation to: ${phoneNumber}, conversation ID: ${conversationId}`);
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing triple-check after lock may introduce race conditions.
Please confirm how duplicate conversation creation is prevented, or specify where locking is enforced if handled elsewhere.
</issue_to_address>
### Comment 5
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:1386` </location>
<code_context>
- body?.conversation?.messages[0]?.source_id?.substring(0, 5) === 'WAID:' &&
- body?.conversation?.messages[0]?.id === body?.id
- ) {
+ if (body?.conversation?.messages[0]?.source_id?.substring(0, 5) === 'WAID:') {
return { message: 'bot' };
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Relaxing outgoing message check may allow false positives.
Please verify that removing the message ID comparison does not cause the bot to respond to messages it shouldn't.
</issue_to_address>
### Comment 6
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:2025` </location>
<code_context>
- }
+ ? body.key.participantAlt.split('@')[0].split(':')[0]
+ : body.key.participant.split('@')[0].split(':')[0];
+ const formattedPhoneNumber = parsePhoneNumberFromString(`+${rawPhoneNumber}`).formatInternational();
let content: string;
</code_context>
<issue_to_address>
**issue:** Using libphonenumber-js for formatting improves consistency but may throw.
Handle cases where parsePhoneNumberFromString returns undefined or throws to prevent runtime errors.
</issue_to_address>
### Comment 7
<location> `src/utils/onWhatsappCache.ts:120` </location>
<code_context>
- },
- });
- }
+ await prismaRepository.isOnWhatsapp.upsert({
+ where: { remoteJid: remoteJid },
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Switching to upsert simplifies cache logic and improves atomicity.
Please verify that the database enforces a unique constraint on remoteJid to maintain data integrity.
Suggested implementation:
```typescript
/**
* NOTE: Ensure that the database enforces a unique constraint on the `remoteJid` field
* in the `isOnWhatsapp` table to guarantee upsert atomicity and data integrity.
* For Prisma, add `@unique` to the `remoteJid` field in your schema.prisma file:
*
* model isOnWhatsapp {
* id Int @id @default(autoincrement())
* remoteJid String @unique
* // ... other fields ...
* }
*/
```
You must also update your Prisma schema (usually in `prisma/schema.prisma`) to add `@unique` to the `remoteJid` field in the `isOnWhatsapp` model, then run `npx prisma migrate dev` to apply the migration.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const conversationId = (await this.cache.get(cacheKey)) as number; | ||
| this.logger.verbose(`Found conversation to: ${phoneNumber}, conversation ID: ${conversationId}`); | ||
| let conversationExists: conversation | boolean; | ||
| let conversationExists: any; |
Contributor
There was a problem hiding this comment.
suggestion: Switching conversationExists type to any reduces type safety.
Consider using a union type or a descriptive type alias if the value can be multiple types, rather than 'any', to maintain type safety and clarity.
Suggested implementation:
type ConversationOrFalse = conversation | false;
let conversationExists: ConversationOrFalse;- If the
conversationtype is not already imported or defined, ensure it is imported from the correct module. - If you prefer, you can define the type alias at the top of the file or near other type definitions for better visibility.
| this.logger.verbose(`Processing group conversation`); | ||
| const group = await this.waMonitor.waInstances[instance.instanceName].client.groupMetadata(chatId); | ||
| this.logger.verbose(`Group metadata: ${JSON.stringify(group)}`); | ||
| this.logger.verbose(`Group metadata: JID:${group.JID} - Subject:${group?.subject || group?.Name}`); |
Contributor
There was a problem hiding this comment.
nitpick (typo): Potential typo: group.JID may be undefined.
Verify whether the correct property is 'id' instead of 'JID' to avoid logging undefined values.
Suggested change
| this.logger.verbose(`Group metadata: JID:${group.JID} - Subject:${group?.subject || group?.Name}`); | |
| this.logger.verbose(`Group metadata: ID:${group.id} - Subject:${group?.subject || group?.Name}`); |
| } | ||
| ? body.key.participantAlt.split('@')[0].split(':')[0] | ||
| : body.key.participant.split('@')[0].split(':')[0]; | ||
| const formattedPhoneNumber = parsePhoneNumberFromString(`+${rawPhoneNumber}`).formatInternational(); |
Contributor
There was a problem hiding this comment.
issue: Using libphonenumber-js for formatting improves consistency but may throw.
Handle cases where parsePhoneNumberFromString returns undefined or throws to prevent runtime errors.
| }, | ||
| }); | ||
| } | ||
| await prismaRepository.isOnWhatsapp.upsert({ |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Switching to upsert simplifies cache logic and improves atomicity.
Please verify that the database enforces a unique constraint on remoteJid to maintain data integrity.
Suggested implementation:
/**
* NOTE: Ensure that the database enforces a unique constraint on the `remoteJid` field
* in the `isOnWhatsapp` table to guarantee upsert atomicity and data integrity.
* For Prisma, add `@unique` to the `remoteJid` field in your schema.prisma file:
*
* model isOnWhatsapp {
* id Int @id @default(autoincrement())
* remoteJid String @unique
* // ... other fields ...
* }
*/You must also update your Prisma schema (usually in prisma/schema.prisma) to add @unique to the remoteJid field in the isOnWhatsapp model, then run npx prisma migrate dev to apply the migration.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } else { | ||
| const { host, password, port, protocol: proto, username } = proxy | ||
| protocol = (proto || 'http').replace(':', '') | ||
| const { host, password, port, protocol: proto, username } = proxy; |
Check failure
Code scanning / CodeQL
Insecure randomness High
| } else { | ||
| const { host, password, port, protocol: proto, username } = proxy | ||
| protocol = (proto || 'http').replace(':', '') | ||
| const { host, password, port, protocol: proto, username } = proxy; |
Check failure
Code scanning / CodeQL
Insecure randomness High
| } else { | ||
| const { host, password, port, protocol: proto, username } = proxy | ||
| protocol = (proto || 'http').replace(':', '') | ||
| const { host, password, port, protocol: proto, username } = proxy; |
Check failure
Code scanning / CodeQL
Insecure randomness High
| } else { | ||
| const { host, password, port, protocol: proto, username } = proxy | ||
| protocol = (proto || 'http').replace(':', '') | ||
| const { host, password, port, protocol: proto, username } = proxy; |
Check failure
Code scanning / CodeQL
Insecure randomness High
| } else { | ||
| const { host, password, port, protocol: proto, username } = proxy | ||
| protocol = (proto || 'http').replace(':', '') | ||
| const { host, password, port, protocol: proto, username } = proxy; |
Check failure
Code scanning / CodeQL
Insecure randomness High
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by Sourcery
Improve integration between Chatwoot and Baileys services by fixing identifier mapping, standardizing phone formatting, enriching logs, refactoring cache and startup logic, extending DTOs with connectionStatus/ownerJid, and bumping the package version
New Features:
Bug Fixes:
Enhancements: