Improve/error handling #143
Conversation
…o improve/error-handling
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @abdheshnayak - I've reviewed your changes and they look great!
General suggestions:
- Consider implementing an exponential backoff strategy for socket reconnections to improve robustness in unstable network conditions.
- Review the use of ESLint rule disabling to ensure it doesn't hide potential issues, especially regarding naming conventions.
- Ensure consistent error logging practices for better debugging and maintenance.
- Address the typo in the function name to improve code clarity.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| const connnect = (recon = () => {}) => { | ||
| try { | ||
| sockPromise.current = new Promise<wsock.w3cwebsocket>( | ||
| (res, rej) => { | ||
| try { | ||
| // eslint-disable-next-line new-cap | ||
| const w = new wsock.w3cwebsocket( | ||
| `${socketUrl}/ws`, | ||
| '', | ||
| '', | ||
| {} | ||
| ); | ||
|
|
||
| w.onmessage = onMessage; | ||
|
|
||
| w.onopen = () => { | ||
| res(w); | ||
| }; | ||
|
|
||
| w.onerror = (e) => { | ||
| console.error(e); | ||
| recon(); | ||
| }; | ||
|
|
||
| w.onclose = () => { | ||
| recon(); | ||
| }; | ||
| } catch (e) { | ||
| rej(e); | ||
| } | ||
| }; | ||
| } | ||
| ); | ||
| } catch (e) { | ||
| logger.error(e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
nitpick (typo): The function name 'connnect' seems to be a typo. Consider renaming it to 'connect' for clarity.
| sockPromise.current = new Promise<wsock.w3cwebsocket>( | ||
| (res, rej) => { | ||
| try { | ||
| // eslint-disable-next-line new-cap |
There was a problem hiding this comment.
suggestion (code_refinement): Disabling ESLint rules can hide potential issues. If the 'new-cap' rule is frequently disabled, consider configuring this rule project-wide or ensuring that the naming conventions for constructors are followed.
| w.onerror = (e) => { | ||
| console.error(e); | ||
| recon(); |
There was a problem hiding this comment.
suggestion (code_refinement): Logging the error before calling the reconnect function could provide valuable debugging information. Consider logging the error using logger.error or console.error before the recon() call.
| connnect(() => { | ||
| setTimeout(() => { | ||
| console.log('reconnecting'); | ||
| connnect(); | ||
| }, 1000); | ||
| }); |
There was a problem hiding this comment.
suggestion (performance): Implementing an exponential backoff strategy for reconnection attempts could improve the robustness of the socket connection, especially in unstable network conditions. Constantly reconnecting every 1 second might not be the most efficient approach.
| @@ -747,7 +747,7 @@ const LogComp = ({ | |||
| {isLoading && <LoadingComp />} | |||
|
|
|||
| {errors.length ? ( | |||
| <pre>{JSON.stringify(errors)}</pre> | |||
| <pre>{JSON.stringify(errors, null, 2)}</pre> | |||
There was a problem hiding this comment.
suggestion (code_refinement): Formatting the errors with JSON.stringify(errors, null, 2) improves readability in the UI. This is a good practice for enhancing the debugging experience.
* 🎨 Improved error handling for serverside
* 🎨 Improved error handling for serverside
* 🎨 Improved error handling for serverside
No description provided.