fix: delete instance cleanup on baileys failure

[antonio-abrantes]

📋 Description

🐛 Problem

When calling DELETE /instance/delete/{instanceName} on a corrupted
instance (disconnected/bugged Baileys session), the API returns:

{
  "status": 400,
  "error": "Bad Request",
  "response": { "message": ["[object Object]"] }
}

After this error, the instance name remains permanently blocked in
waInstances memory. Even after manually cleaning the database, Redis,
and session files, calling POST /instance/create with the same name
returns:

{
  "status": 403,
  "message": ["This name \"agent-xxx\" is already in use."]
}

The only workaround was restarting the container — not viable in production environments.

---

🔍 Root Cause

Two bugs working together:

1. Aborted cleanup

• deleteInstance() stopped execution if logout() failed

2. Bad error serialization

• Errors returned as [object Object]

---

✅ Fix

• Split try/catch into isolated steps

• Ensure cleanup always runs

• Always remove instance from waInstances

• Fix error serialization using:

  error?.message ?? String(error)
  

---

🔗 Related Issue

Closes # (leave empty if not applicable)

---

🧪 Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ New feature
• 💥 Breaking change
• 📚 Documentation update
• 🔧 Refactoring
• ⚡ Performance improvement
• 🧹 Code cleanup
• 🔒 Security fix

---

🧪 Testing

• Manual testing completed
• Functionality verified in development environment
• No breaking changes introduced
• Tested with different connection types

---

📸 Screenshots (if applicable)

N/A

---

✅ Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• My changes generate no new warnings
• I have manually tested my changes thoroughly
• I have verified the changes work with different scenarios

---

📝 Additional Notes

Tested in production environment with Docker Swarm + PostgreSQL + Redis.
Fix only affects failure path — no impact on healthy instances.

---

⚠️ Behavior Change — Contract Documentation

The endpoint now returns { status: 'SUCCESS' } even on partial failures.
Logout and webhook steps were changed from "abort on error" to logger.warn.
This is intentional — the cleanup is best-effort to ensure the instance
name is always released from waInstances memory, even when secondary steps fail.

Clients relying on 4xx responses to detect cleanup failures will need to
adjust their logic.

[sourcery-ai]

Sorry @antonio-abrantes, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[DavidsonGomes]

Hi! As part of our PR triage, we re-ran CI on this PR and the Check Code Quality workflow is failing. Could you please:

1. Check the latest run output (Actions tab on this PR) and address the build/lint errors
2. Push the fix to your branch — CI will re-run automatically (no need to ping us; it's now auto-approved)
3. Make sure the base branch is develop (already adjusted on our side)

Once CI is green I'll re-review for merge. Thanks!

[DavidsonGomes]

Boa refatoração — quebrar o try/catch monolítico em steps isolados é a forma idiomática correta de cleanup robusto, e adicionar await em cleaningUp e cleaningStoreData corrige uma race condition latente.

Bloqueadores para o merge:

• Rebase — PR está CONFLITANDO com main
• Rodar npm run lint -- --fix — CI falha com 85 erros prettier em monitor.service.ts (linhas 1850-2791, formatação automática)
• Documentar mudança de contrato no PR body: o endpoint passa a retornar { status: 'SUCCESS' } mesmo em falhas parciais (logout/webhook viram logger.warn em vez de abortar). Isso é uma escolha deliberada — o cleanup é "best effort" para liberar o nome da instância — mas merece estar explícito para clientes que dependiam de 4xx.

Após os 3 itens, aprovo.

[DavidsonGomes]

Olá @antonio-abrantes, obrigado pela contribuição — o fix está correto e endereça um problema real de produção (instâncias travadas no waInstances em memória após falha de cleanup).

A PR está em estado CONFLICTING com develop. Pode fazer rebase contra develop quando puder?

git fetch upstream
git rebase upstream/develop
git push --force-with-lease

Após o rebase, mergeio. Obrigado!

[NeritonDias]

🟢 👍

Na develop e focado: melhora o cleanup ao deletar instância quando o Baileys falha — isola o try/catch por etapa e adiciona os await que faltavam, sem reescrever a lógica. Pode seguir.

@dpaes

[dpaes]

Approved.

Reviewed independently against develop:

• Real bug fixed: in the original removeInstance the try had only a finally (no catch). If sendDataWebhook(REMOVE_INSTANCE) threw (dead Baileys socket), the exception propagated and skipped cleaningUp/cleaningStoreData and the delete this.waInstances[instanceName], leaving a stale in-memory entry until the process restarted. Same pattern in the controller's deleteInstance. Isolating each step into its own try/catch and guaranteeing the delete/emit always run is the correct fix.
• The awaits added to cleaningUp/cleaningStoreData (both async) are right: they were fire-and-forget, causing unhandled rejections and a race with the subsequent delete. The bodies only do idempotent cleanup (deleteMany, rmSync, cache.delete), so sequential await doesn't change success semantics — it just makes it deterministic. Deletion still proceeds even if one step fails.

Focused and additive, no regression. LGTM 👍

---

Non-blocking suggestions for a follow-up commit (optional, include if you want — none block this merge):

1. clearDelInstanceTime is left outside a try/catch in the monitor service. It's synchronous and trivial (and it had no dedicated catch in the original either), but if it ever throws synchronously it would abort the rest of the cleanup inside the async listener — wrapping it for consistency would be tidier.
2. sendDataWebhook in the controller's deleteInstance still has no await inside its try/catch — if it returns a rejecting promise, the synchronous catch won't capture it. Identical to the original (not a regression), just not hardened like the other steps.
3. error.toString() → error?.message ?? String(error): nice safety improvement (avoids a TypeError when error is null/undefined). Slightly tangential to the PR's theme but on the same axis (safe error stringify) and trivial, so fine to keep.