Skip to content

Conversation

@z-Fng
Copy link
Member

@z-Fng z-Fng commented Nov 4, 2025

Motivation and Context

When Aria2c download fails, it prompts the user to create a GitHub issue, but the default downloader doesn't show any prompt.

Description

This PR makes the following changes:

  • feat(download|scoop-download): Add GitHub issue prompt when the default downloader fails
  • fix(download): Skip GitHub issue prompt when falling back to the default downloader

How Has This Been Tested?

scoop install games/openxcom
scoop download games/openxcom
  • Before:
    image
  • After
    image

Checklist

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • New Features

    • Download failure messages now include a GitHub issue prompt to help users report problems when a download fails.
  • Bug Fixes

    • Improved invalid-download handling: explicit error output shown for invalid URLs and the issue prompt is surfaced when the primary or fallback downloader fails.

@coderabbitai
Copy link

coderabbitai bot commented Nov 4, 2025

Walkthrough

Adds explicit error emission "URL is not valid" and then aborts with a structured GitHub-issue message via new_issue_msg($app, $bucket, 'download failed') in download failure handlers (primary and aria2 fallback); normalizes some status-message quoting. No public APIs changed. (≈33 words)

Changes

Cohort / File(s) Change Summary
Changelog Update
CHANGELOG.md
Added Unreleased Features entry documenting that a GitHub issue prompt is shown when the default downloader fails
Download error handlers
lib/download.ps1
On download failures (primary and aria2 fallback), emit Error "URL $url is not valid" before aborting with new_issue_msg($app, $bucket, 'download failed'); removed inline warning in aria2 inner catch; normalized status-message quoting to single quotes
Download wrapper
libexec/scoop-download.ps1
When URL is invalid, emit the invalid-URL error and then log/abort with new_issue_msg($app, $bucket, 'download failed'); no other control-flow changes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Default as DefaultDownloader
  participant Aria2 as aria2 (fallback)
  participant Issue as new_issue_msg
  participant Abort

  User->>Default: request download(URL)
  alt download succeeds
    Default-->>User: file saved
  else download fails / URL invalid
    Default-->>User: Emit Error "URL <url> is not valid"
    Default->>Issue: new_issue_msg(app, bucket, 'download failed')
    Issue-->>Abort: structured issue message
    Abort-->>User: abort with issue message
  else Default fails -> fallback to aria2
    Default->>Aria2: attempt download
    alt aria2 succeeds
      Aria2-->>User: file saved
    else aria2 fails
      Aria2-->>User: Emit Error "URL <url> is not valid"
      Aria2->>Issue: new_issue_msg(app, bucket, 'download failed')
      Issue-->>Abort: structured issue message
      Abort-->>User: abort with issue message
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify new_issue_msg signature/escaping and that message formatting matches callers.
  • Confirm error emission happens before abort in all relevant catch blocks.
  • Check aria2 fallback inner catch for identical semantics and no behaviour regressions.

Possibly related PRs

Poem

🐰 I hopped through handlers, nose to the log,
Found a URL lost in the fog.
"An error," I said, "and a ticket to file,"
So humans can fix it with patience and style. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding a GitHub issue prompt when the default downloader fails, which aligns with the PR objectives and file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c43d947 and 9660231.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • lib/download.ps1 (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/download.ps1
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T01:48:00.222Z
Learnt from: z-Fng
Repo: ScoopInstaller/Scoop PR: 6471
File: CHANGELOG.md:9-9
Timestamp: 2025-08-31T01:48:00.222Z
Learning: The Scoop project's CHANGELOG.md follows a convention of tracking PR numbers only, not issue numbers, according to the maintainer z-Fng.

Applied to files:

  • CHANGELOG.md
🔇 Additional comments (1)
CHANGELOG.md (1)

7-7: Entry is well-formatted and follows project conventions.

The CHANGELOG entry correctly documents the new feature with proper scope, description, and PR reference. Formatting and placement are consistent with established patterns in the file, and the PR number convention is correctly applied.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants