fix(sync): added client.wait_for_start() and improve error handling in sync daemon

[ErikBjare]

• Added error logging in daemon before re-throwing errors
• Implemented exponential backoff when checking if the server is running
• Addresses issue New modules fail to start at launch aw-qt#105

---

Important

Improves error handling in aw-sync daemon and adds exponential backoff for server connections in main.rs and sync_wrapper.rs.

• Error Handling:
  • Added error logging in daemon() in main.rs before re-throwing errors.
  • Introduced daemon_sync_cycle() in main.rs to encapsulate sync operations and handle errors.
• Server Connection:
  • Implemented wait_for_start() in aw-client-rust/src/lib.rs for exponential backoff when connecting to the server.
  • Replaced direct server connection check in pull() in sync_wrapper.rs with wait_for_start().
• Issue Addressed:
  • Addresses issue #105.

^{This description was created by for 46b586e. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 9b0d59f in 45 seconds

More details

• Looked at 101 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. aw-sync/src/sync_wrapper.rs:35

• Draft comment:
  The error message should reflect the actual total wait time instead of a fixed 10 seconds. Consider using total_wait in the message.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment is about a change made in the diff, specifically the error message in the wait_for_server function. The suggestion to use total_wait in the error message is actionable and improves the accuracy of the error message, which is a valid code quality improvement.
  The comment might be considered minor since it only affects the error message, not the logic. However, accurate error messages are important for debugging.
  Improving error messages is a valid concern as it aids in debugging and understanding the issue when it occurs.
  The comment is valid and should be kept as it suggests an actionable improvement to the error message.

2. aw-sync/src/sync_wrapper.rs:24

• Draft comment:
  connect_timeout should use a fixed timeout for each connection attempt, not the increasing retry_delay. Consider using a separate fixed timeout value for connect_timeout.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment is suggesting a change in the logic of how timeouts are handled in the wait_for_server function. The current implementation uses an exponential backoff strategy, which is a common approach for retry mechanisms. The suggestion to use a fixed timeout for each attempt could be a valid point if the exponential backoff is not appropriate for this use case. However, without more context on the intended behavior, it's hard to say definitively if this change is necessary.
  I might be missing the context of why exponential backoff was chosen in the first place. It could be that the author intended to use this strategy for a specific reason, such as reducing load on the server or network.
  The comment does raise a valid point about the potential misuse of connect_timeout with an increasing delay, which could lead to unexpected behavior. It's worth considering if the current implementation aligns with the intended use case.
  The comment raises a valid concern about the use of connect_timeout with an increasing delay, which could lead to unexpected behavior. It should be kept for the author to consider if the current implementation aligns with the intended use case.

Workflow ID: wflow_kfoHpwCDSz9YrSKh

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 42cb5d8 in 15 seconds

More details

• Looked at 43 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. aw-sync/src/sync_wrapper.rs:24

• Draft comment:
  The connect_timeout function should not use retry_delay as its timeout. Instead, use a fixed timeout for each connection attempt and use retry_delay only for the sleep duration between attempts. This ensures that each connection attempt has a reasonable timeout, regardless of the retry delay.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_qLiWQKZvrM9mjcwh

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on 4723f32 in 24 seconds

More details

• Looked at 113 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. aw-client-rust/src/blocking.rs:83

• Draft comment:
  Consider making wait_for_server an async function to avoid blocking the thread, especially since the rest of the AwClient methods are async.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_NonIZFhTd36fJxxx

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 451367f in 10 seconds

More details

• Looked at 28 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. aw-client-rust/src/lib.rs:227

• Draft comment:
  Consider updating the comment to reflect the new function name wait_for_start instead of server. This will maintain consistency and avoid confusion.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The renaming of the function from wait_for_server to wait_for_start is consistent across both files. However, the comment in aw-client-rust/src/lib.rs still mentions server instead of start. This could lead to confusion.

Workflow ID: wflow_J0byOdZ9Ag5n3Cbh

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on ed15ff5 in 9 seconds

More details

• Looked at 50 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. aw-client-rust/src/blocking.rs:84

• Draft comment:
  The function name wait_for_start is less descriptive than wait_for_server. Consider reverting to wait_for_server for clarity.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The change from wait_for_server to wait_for_start is consistent across files, but the function name wait_for_start is less descriptive than wait_for_server. The original name clearly indicated the purpose of the function, which is to wait for the server to be ready. The new name could be misleading as it might imply waiting for the application to start rather than the server. This change affects multiple files, including aw-client-rust/src/blocking.rs, aw-client-rust/src/lib.rs, and aw-sync/src/sync_wrapper.rs.

Workflow ID: wflow_tQo6iTnnTmqLItGc

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 46b586e in 13 seconds

More details

• Looked at 54 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. aw-client-rust/src/lib.rs:228

• Draft comment:
  The wait_for_start function is not async, but the comment suggests it should be. Consider implementing it as an async function if possible.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR changes the function name from wait_for_server to wait_for_start in aw-client-rust/src/blocking.rs and aw-client-rust/src/lib.rs. However, the function wait_for_start in aw-client-rust/src/lib.rs is not async, and the comment // TODO: make async suggests it should be. This could be a potential issue if the function is expected to be async in the future.

2. aw-sync/src/sync_wrapper.rs:16

• Draft comment:
  The function call wait_for_server is updated to wait_for_start, which is consistent with the changes in aw-client-rust/src/blocking.rs.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The PR changes the function name from wait_for_server to wait_for_start in aw-client-rust/src/blocking.rs. The function call in aw-sync/src/sync_wrapper.rs is also updated to wait_for_start. This is consistent with the changes made in the PR.

Workflow ID: wflow_nFwxbj7ETtfh0Zlu

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.