-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix Chatwoot DB Connection Instability and Implement Stale Conversation Cache Handling #2017
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 Chatwoot DB Connection Instability and Implement Stale Conversation Cache Handling #2017
Conversation
…ento de mensagens - Alterado método de obtenção da conexão PostgreSQL para ser assíncrono, melhorando a gestão de conexões. - Implementada lógica de retry para criação de mensagens e conversas, garantindo maior robustez em caso de falhas. - Ajustadas chamadas de consulta ao banco de dados para utilizar a nova abordagem de conexão. - Adicionada nova propriedade `messageBodyForRetry` para facilitar o reenvio de mensagens em caso de erro.
Reviewer's GuideThis PR refactors ChatwootService to initialize the PostgreSQL client on-demand via getPgClient to fix intermittent connection errors and adds a robust retry mechanism for stale conversation IDs: both createMessage and sendData flows now catch 404 errors, purge the invalid cache entry, create a fresh conversation, and retry the operation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Consider abstracting the retry logic for stale-conversation handling into a shared helper to reduce duplication between createMessage and sendData.
- Instead of calling getPgClient() for every query, acquire the client once per operation and reuse it to minimize overhead and simplify the code paths.
- Relying on error.toString().toLowerCase() to detect 404s can be brittle—use AxiosError.response.status or error.isAxiosError to more reliably handle HTTP status codes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider abstracting the retry logic for stale-conversation handling into a shared helper to reduce duplication between createMessage and sendData.
- Instead of calling getPgClient() for every query, acquire the client once per operation and reuse it to minimize overhead and simplify the code paths.
- Relying on error.toString().toLowerCase() to detect 404s can be brittle—use AxiosError.response.status or error.isAxiosError to more reliably handle HTTP status codes.
## Individual Comments
### Comment 1
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:919` </location>
<code_context>
const remoteJid = bodyForRetry.key.remoteJid;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {remoteJid} = bodyForRetry.key;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 2
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:1101` </location>
<code_context>
const remoteJid = bodyForRetry.key.remoteJid;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {remoteJid} = bodyForRetry.key;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts
Outdated
Show resolved
Hide resolved
src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts
Outdated
Show resolved
Hide resolved
…e.ts aplicação de desestruturação de objetos que é uma boa prática do ts Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…e.ts aplicação de desestruturação de objetos que é uma boa prática do ts Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
- Implementada a função `handleStaleConversationError` para centralizar a lógica de tratamento de erros relacionados a conversas não encontradas. - A lógica de retry foi aprimorada para as funções `createMessage` e `sendData`, garantindo que as operações sejam reprocessadas corretamente em caso de falhas. - Removido código duplicado e melhorada a legibilidade do serviço Chatwoot.
|
@Vitordotpy what issues are you experiencing because of this bug? I'm having issue where certain instances won't send messages to Chatwoot, but Chatwoot can send messages to Whatsapp. Wondering if this is related to it and solves it. |
@andres99x I'm currently experiencing race conditions when Evolution tries to access the Chatwoot database, as well as other issues caused by the Redis cache, which causes an error when Evolution tries to send a message to a contact whose conversations were previously deleted. Evolution tries to use cached conversations that don't exist and fails to send the messages to Chatwoot. My solution was to add a retry when sending messages to Chatwoot. If the Redis cache is out of date, the Chatwoot service will retry without cached data. |
This pull request addresses two critical issues in the Chatwoot integration to improve its reliability and resilience. It introduces on-demand database connections to prevent intermittent connection failures and implements a retry mechanism to handle stale conversation data cached in Redis.
Fixes:
ChatwootServiceto initialize the PostgreSQL client on-demand (getPgClient) rather than at service instantiation. This resolves a race condition where the client could be created with an incorrect or default database connection string, causing sporadicdatabase does not existerrors.try-catchmechanism within thecreateMessageandsendDatamethods. When a404 Not Founderror is received from the Chatwoot API (indicating a stale conversation ID in the Redis cache), the system now automatically:Enhancements:
Summary
This PR resolves critical stability issues in the Chatwoot integration by implementing on-demand database connections to fix intermittent connection errors and adding a retry mechanism that automatically handles stale conversation IDs from the Redis cache, ensuring more reliable message delivery.
Summary by Sourcery
Fix intermittent database connection errors and add automatic retry handling for stale Chatwoot conversation IDs by lazily initializing the Postgres client and evicting and recreating conversations on 404 errors.
Bug Fixes:
Enhancements: