Skip to content

Fix task error handling and deduplicate task ID validation#91

Merged
manthanabc merged 4 commits intopaws-servefrom
copilot/sub-pr-90
Feb 14, 2026
Merged

Fix task error handling and deduplicate task ID validation#91
manthanabc merged 4 commits intopaws-servefrom
copilot/sub-pr-90

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 14, 2026

Addresses two code quality issues identified in PR review: tasks with stream errors were incorrectly marked as Completed, and task ID validation logic was duplicated across 6 handlers.

Changes

Error handling in task execution

  • Track stream errors during task execution and mark tasks as Failed instead of Completed when errors occur
  • Prevents conflicting terminal states where error events are emitted but task shows as successfully completed

Task ID validation

  • Extract duplicated validation logic into shared parse_task_id() helper in handlers/utils.rs
  • Consolidates validation across get_task, cancel_task, get_task_events, get_task_events_since, stream_task_events, and stream_task_events_resumable
  • Adds unit tests for edge cases (undefined, empty, invalid UUID)

Example

Before:

// Duplicated in 6 places
if id == "undefined" || id.is_empty() {
    return Err(AppError::bad_request("..."));
}
let task_id: TaskId = id.parse().map_err(|e| ...)?;

After:

// Single shared implementation
let task_id = parse_task_id(&id)?;

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits February 14, 2026 19:54
Co-Authored-By: Paws <noreply@pawscode.dev>

Co-authored-by: manthanabc <48511543+manthanabc@users.noreply.github.com>
Co-Authored-By: Paws <noreply@pawscode.dev>

Co-authored-by: manthanabc <48511543+manthanabc@users.noreply.github.com>
Co-Authored-By: Paws <noreply@pawscode.dev>

Co-authored-by: manthanabc <48511543+manthanabc@users.noreply.github.com>
Copilot AI changed the title [WIP] Add API for WebUI with task orchestration and SSE streaming Fix task error handling and deduplicate task ID validation Feb 14, 2026
Copilot AI requested a review from manthanabc February 14, 2026 20:01
@manthanabc manthanabc marked this pull request as ready for review February 14, 2026 20:47
Copilot AI review requested due to automatic review settings February 14, 2026 20:47
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@manthanabc manthanabc merged commit 321e550 into paws-serve Feb 14, 2026
2 checks passed
@manthanabc manthanabc deleted the copilot/sub-pr-90 branch February 14, 2026 20:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves task lifecycle correctness in paws_server by ensuring tasks that encounter streaming errors are terminally marked as Failed, and it reduces handler duplication by centralizing task ID validation into a shared helper.

Changes:

  • Track stream errors during task execution and emit Failed (instead of Completed) when any stream error occurs.
  • Add handlers::parse_task_id() to consolidate task ID validation/parsing across task and SSE endpoints.
  • Add unit tests for parse_task_id() edge cases.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/paws_server/src/task/manager.rs Track stream errors and set terminal task status/event to Failed when errors occur.
crates/paws_server/src/handlers/utils.rs New shared parse_task_id() helper plus unit tests.
crates/paws_server/src/handlers/tasks.rs Replace duplicated ID validation/parsing with parse_task_id().
crates/paws_server/src/handlers/sse.rs Replace duplicated ID validation/parsing with parse_task_id().
crates/paws_server/src/handlers/mod.rs Register and re-export the new utils handler module.
Cargo.lock Lockfile update reflecting dependency resolution (hyper entry).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +29 to +30
use pretty_assertions::assert_eq;

Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test module imports pretty_assertions::assert_eq but never uses it, which triggers an unused-import warning in tests. Remove the import or update the assertions to use it.

Suggested change
use pretty_assertions::assert_eq;

Copilot uses AI. Check for mistakes.
@@ -9,6 +9,7 @@ use tokio_stream::wrappers::BroadcastStream;
use crate::AppError;
use crate::server::AppState;
use crate::task::TaskId;
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TaskId is imported but not used in this module after switching to parse_task_id, which will trigger an unused-import warning. Remove the use crate::task::TaskId; import.

Suggested change
use crate::task::TaskId;

Copilot uses AI. Check for mistakes.
Comment on lines +161 to +165
if has_error {
// Safety: has_error is only set to true when last_error is Some
let error_msg = last_error.unwrap();
task.fail(error_msg.clone());
store.update_task(task.clone()).await;
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last_error.unwrap() can panic if the invariants around has_error/last_error ever change (e.g., future refactors). You can simplify and harden this by removing has_error and branching on if let Some(error_msg) = last_error instead (or use expect with a clear message).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants