[BUG] Unguarded error.message access in catch blocks causes secondary TypeErrors when non-Error values are thrown

[0xMink]

Problem (one or two sentences)

This report covers two related but independent issues in ClineProvider.ts error handling:

1. Unguarded error.message access (5 locations) — runtime safety
2. Contradictory non-null assertion in cancelTask() (1 location) — logic contradiction

These can be addressed in separate commits if preferred.

Several catch blocks in ClineProvider.ts access error.message directly without narrowing the caught value. If a non-Error is thrown (string, object, null), this causes secondary TypeErrors or blank error dialogs instead of useful error reporting. Additionally, cancelTask() uses this.getCurrentTask()! === undefined which is internally contradictory (non-null assertion followed by an undefined check).

Context (who is affected and when)

Affects any user when an API call, task abort, or marketplace fetch throws a non-Error value (e.g., a string or plain object). This can happen with third-party API providers, network failures, or unexpected runtime conditions. The user sees a blank error dialog or a "Cannot read properties of undefined (reading 'message')" error instead of the actual failure reason.

Note: This is separate from the unsafe full error-object serialization issue; this report is about runtime safety and correctness of error handling.

Reproduction steps

1. Environment: Any OS, Roo Code v3.47.3, any API provider.

2. Unguarded .message access in catch blocks (search pattern: catch blocks using error.message where the caught value is untyped/unknown):

   • performPreparationTasks (~line 449): vscode.window.showErrorMessage(error.message)
   • removeClineFromStack (~line 473): ${e.message}
   • createTaskWithHistoryItem (~line 1006): ${e.message}
   • fetchMarketplaceData (~line 1892): errors: [error.message]
   • resumeTask (~line 3019): ${error.message}

3. If any of these paths receives a thrown string/plain object (or null) instead of an Error instance, .message may be undefined, producing a blank/misleading log or a secondary TypeError.

4. Separately, cancelTask() (~line 2967) uses this.getCurrentTask()! === undefined (non-null assertion immediately compared to undefined), which is logically contradictory and can be refactored into a clean nullish check.

Expected result

Caught values are narrowed before .message access (for example: err instanceof Error ? err.message : String(err)), and cancelTask() uses a single const current = this.getCurrentTask() check without non-null assertions.

Actual result

Direct .message access on a non-Error thrown value can produce secondary TypeErrors or blank error messages, obscuring the original failure. this.getCurrentTask()! === undefined is internally contradictory and should be simplified.

Variations tried (optional)

The pattern error instanceof Error ? error.message : String(error) is already used in other parts of the same file, confirming it is the established convention. The unguarded locations were likely missed during earlier refactors.

App Version

v3.47.3

API Provider (optional)

None

Model Used (optional)

N/A

Roo Code Task Links (optional)

No response

Relevant logs or errors (optional)

Search pattern for affected locations: error.message in catch blocks where the caught value is untyped/unknown
Search pattern for non-null assertion: this.getCurrentTask()! === undefined
Affected file: src/core/webview/ClineProvider.ts

[roomote]

I've analyzed this issue and here's my proposed implementation plan:

File: src/core/webview/ClineProvider.ts

1. Guard 5 unguarded error.message accesses using the existing instanceof Error pattern

Apply error instanceof Error ? error.message : String(error) at each location:

• Line 449 (performPreparationTasks): vscode.window.showErrorMessage(error.message) -- the catch variable error is untyped
• Line 473 (removeClineFromStack): `...${e.message}` -- catch variable e is untyped
• Line 1007 (createTaskWithHistoryItem): `...${e.message}` -- catch variable e is untyped
• Line 1900 (fetchMarketplaceData): errors: [error.message] -- .catch() callback error is untyped
• Line 3030 (resumeTask): `...${error.message}` -- .catch() callback error is untyped

Note: Line 731 is already safe (inside an instanceof OrganizationAllowListViolationError guard).

2. Fix the contradictory non-null assertion in cancelTask()

Lines 2976-2984 use this.getCurrentTask()! (non-null assertion) while simultaneously comparing to undefined, which is contradictory. Also calls getCurrentTask() 4 times redundantly.

Replace:

await pWaitFor(
  () =>
    this.getCurrentTask()! === undefined ||
    this.getCurrentTask()!.isStreaming === false ||
    this.getCurrentTask()!.didFinishAbortingStream ||
    this.getCurrentTask()!.isWaitingForFirstChunk,
  { timeout: 3_000 },
)

With:

await pWaitFor(
  () => {
    const current = this.getCurrentTask()
    return (
      current === undefined ||
      current.isStreaming === false ||
      current.didFinishAbortingStream ||
      current.isWaitingForFirstChunk
    )
  },
  { timeout: 3_000 },
)

3. Add/update tests

Add test coverage to verify the error-handling changes behave correctly.

This approach follows the established pattern already used elsewhere in the same file and addresses both the runtime safety and the logic contradiction.

Please mention @roomote in a comment saying that you approve this plan, and I'll proceed with the implementation.

[0xMink]

Thanks for the analysis — this matches what I found. I already have a working fix with tests ready to go. Claiming
this issue.

@hrudolph requesting assignment.

[0xMink]

Fix is ready and in review. Will link the PR once it's published.