bug proxy crashes #190
bug proxy crashes #190
Conversation
Removed panic(err): Replaced with proper error handling that: Logs the error in detail Cleans resources (clients and connections) Updates the status in the database Sends a webhook notifying of the error Returns normally without crashing the application Proxy error handling: The code already had good handling for invalid URLs, but now with clearer logs Removed another panic: The panic after GetDevice was also replaced by the creation of a new device Benefits: ✅ The application no longer crashes when there is a connection error ✅ The user is notified via webhook about the error ✅ The status in the database is updated correctly ✅ Resources are cleaned properly ✅ Detailed logs for debugging
Summary of ChangesHello @cleitonme, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's stability and error handling capabilities. By replacing Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively removes several panic calls, which will improve the application's stability by preventing crashes on connection errors. The introduction of a retry mechanism with backoff for connection attempts is a great addition for resilience. I've added a few comments with suggestions to improve code clarity and maintainability, such as removing duplicated code, correcting a misleading comment about the backoff strategy, and using constants for magic numbers.
wmiau.go
Outdated
| jid, _ := parseJID(textjid) | ||
| deviceStore, err = container.GetDevice(context.Background(), jid) | ||
| if err != nil { | ||
| panic(err) | ||
| log.Error().Err(err).Msg("Failed to get device") | ||
| deviceStore = container.NewDevice() | ||
| } |
There was a problem hiding this comment.
This block of code (lines 399-409) appears to be a duplicate of an earlier block (lines 370-380). The deviceStore is re-initialized here, but this new value is not used, as the whatsmeow.Client was already created on lines 392/394 with the deviceStore from the first block. This redundant code can be confusing and should be removed to improve maintainability.
wmiau.go
Outdated
| if err != nil { | ||
| panic(err) | ||
|
|
||
| // Retry logic com backoff exponencial |
There was a problem hiding this comment.
This comment is not in English and incorrectly describes the backoff strategy as exponential. The implementation time.Duration(attempt) * 5 * time.Second results in a linear backoff (5s, 10s, ...). Please update the comment to be in English and accurately reflect the linear backoff.
| // Retry logic com backoff exponencial | |
| // Retry logic with linear backoff |
wmiau.go
Outdated
| maxRetries := 3 | ||
| var lastErr error | ||
|
|
||
| for attempt := 0; attempt < maxRetries; attempt++ { | ||
| if attempt > 0 { | ||
| waitTime := time.Duration(attempt) * 5 * time.Second |
There was a problem hiding this comment.
The values 3 for max retries and 5 * time.Second for the backoff interval are hardcoded. It's a good practice to define these 'magic numbers' as named constants (e.g., maxConnectionRetries and connectionRetryBaseWait) at the top of the function or file. This improves readability and makes the code easier to maintain and configure.
wmiau.go
Outdated
| log.Error().Err(dbErr).Msg("Failed to update user status after connection error") | ||
| } | ||
|
|
||
| mycli := MyClient{client, 1, userID, token, subscriptions, s.db, s} |
There was a problem hiding this comment.
The mycli variable is already defined in the outer scope of this function (on line 415). Re-creating an identical instance here is redundant. You can remove this line, and the sendEventWithWebHook call on line 615 will use the existing mycli instance from the outer scope.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great improvement for the application's stability. Replacing the panic calls with proper error handling, especially for connection failures, is a critical fix. The introduction of a retry mechanism with backoff for connection attempts is well-implemented and significantly improves the resilience of the service. The cleanup of resources, database updates, and webhook notifications on failure are all excellent additions.
I have one suggestion to further improve the retry logic by using an exponential backoff strategy, which is generally more robust for such scenarios.
Overall, this is a solid contribution that directly addresses the reported crashes.
|
|
||
| for attempt := 0; attempt < maxConnectionRetries; attempt++ { | ||
| if attempt > 0 { | ||
| waitTime := time.Duration(attempt) * connectionRetryBaseWait |
There was a problem hiding this comment.
While the current linear backoff is a good start, using an exponential backoff is generally a more robust strategy for connection retries. It increases the wait time more significantly with each attempt, which can be more effective during longer service disruptions.
With maxConnectionRetries set to 3, the sequence of wait times is the same as with linear backoff. However, this change implements a true exponential backoff, making the logic more scalable and robust if maxConnectionRetries is increased in the future.
For even greater robustness against 'thundering herd' problems, you might also consider adding a small random jitter to the wait time in a future enhancement.
| waitTime := time.Duration(attempt) * connectionRetryBaseWait | |
| waitTime := connectionRetryBaseWait * time.Duration(1<<(uint(attempt)-1)) |
2 participants
2025-11-01 09:30:41 -03:00 INFO Already logged in, just connect role=wuzapi
panic: couldn't dial whatsapp web websocket: dial tcp 127.0.0.1:5983: connect: connection refused
goroutine 90 [running]:
main.(*server).startClient(0xc00040c980, {0xc0003ba000, 0x20}, {0xc0003ba020, 0x1f}, {0xc0002b6030, 0x24}, {0xc00052af00, 0xb, 0x10})
/home/deploy/WebstormProjects/wuzapi/wmiau.go:537 +0x104d
created by main.(*server).routes.(*server).Connect.func15 in goroutine 86
/home/deploy/WebstormProjects/wuzapi/handlers.go:265 +0x679
Removed panic(err): Replaced with proper error handling that:
Logs the error in detail
Cleans resources (clients and connections)
Updates the status in the database
Sends a webhook notifying of the error
Returns normally without crashing the application
Proxy error handling: The code already had good handling for invalid URLs, but now with clearer logs
Removed another panic: The panic after GetDevice was also replaced by the creation of a new device
Benefits:
✅ The application no longer crashes when there is a connection error ✅ The user is notified via webhook about the error ✅ The status in the database is updated correctly
✅ Resources are cleaned properly
✅ Detailed logs for debugging