fail workflow run when running into cruise initialization errors

[wintonzheng]

Important

Add error handling and logging for observer cruise initialization to fail workflow runs on errors in agent_protocol.py and observer_service.py.

• Error Handling:
  • In agent_protocol.py, wrap initialize_observer_cruise call in try-except to catch LLMProviderError and raise HTTPException with status 500.
  • In observer_service.py, add try-except around create_empty_workflow, setup_workflow_run, update_observer_thought, and update_observer_cruise to log errors and mark workflow runs as failed.
• Logging:
  • Log errors in agent_protocol.py and observer_service.py when exceptions are caught, providing detailed context.
• Workflow Management:
  • Ensure workflow runs are marked as failed in observer_service.py if initialization steps fail, preventing incomplete runs.

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

[ellipsis-dev]

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

More details

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

1. skyvern/forge/sdk/routes/agent_protocol.py:1094

• Draft comment:
  Consider adding logging for the exception before raising HTTPException to ensure all errors are logged for debugging purposes.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The try-except block in initialize_observer_cruise is correctly implemented to catch LLMProviderError and raise an HTTPException. However, the except block should also handle logging the error before raising the exception. This is a common practice to ensure that all errors are logged for debugging purposes.

2. skyvern/forge/sdk/services/observer_service.py:154

• Draft comment:
  Consider adding logging for the exception before raising it to ensure all errors are logged for debugging purposes. This is applicable in other similar try-except blocks as well.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The try-except block in initialize_observer_cruise is correctly implemented to catch LLMProviderError and raise an HTTPException. However, the except block should also handle logging the error before raising the exception. This is a common practice to ensure that all errors are logged for debugging purposes.

Workflow ID: wflow_xWPTqAuiP7uMDQ22

---

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

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to cbc6b48 in 39 seconds

More details

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

1. skyvern/forge/sdk/services/observer_service.py:131

• Draft comment:
  Catching a general Exception can obscure the root cause of errors. Consider catching specific exceptions instead.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  While catching specific exceptions is generally good practice, in this case:

1. This is a database operation where many different types of errors could occur
2. The error handling is just logging a warning and continuing
3. The code is not trying to handle different error types differently
4. The error is non-critical as it's just updating metadata
   The comment has merit as catching specific exceptions is generally better practice. The code could potentially miss critical errors by catching everything.
   However, in this specific case, the broad exception catch is acceptable because it's just logging and continuing execution, and the operation is non-critical metadata update.
   The comment should be deleted as the general Exception catch is appropriate for this non-critical database operation where we just want to log and continue.

2. skyvern/forge/sdk/services/observer_service.py:145

• Draft comment:
  Catching a general Exception can obscure the root cause of errors. Consider catching specific exceptions instead.
• Reason this comment was not posted:
  Marked as duplicate.

3. skyvern/forge/sdk/routes/agent_protocol.py:1087

• Draft comment:
  Catching a general Exception can obscure the root cause of errors. Consider catching specific exceptions instead.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_nYcM07o4wemypduJ

---

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