Skip to content

Conversation

@o-l-a-v
Copy link
Contributor

@o-l-a-v o-l-a-v commented Oct 15, 2025

Closes #16337

  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of Foxit Reader and Foxit PDF Reader downloads by following a single final redirect and producing both 64‑bit and 32‑bit download URLs.
    • Reduced failed installs/updates related to redirect handling while preserving update behavior.
  • Refactor

    • Simplified download-check logic to reduce complexity and improve robustness without changing end-user flow.

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Rewrote checkver logic in both Foxit manifests to perform a single HTTP request via System.Net.HttpWebRequest, use the resolved ResponseUri as the 64-bit download URL, derive the 32-bit URL by replacing "x64" with "x86", and return both URIs joined by a comma. Removed per-architecture loops and previous redirection/Location header handling.

Changes

Cohort / File(s) Summary
Foxit checkver rewrite
bucket/foxit-pdf-reader.json, bucket/foxit-reader.json
Replace PowerShell Invoke-WebRequest looping and Location-header parsing with a single System.Net.HttpWebRequest call that reads .ResponseUri; produce a comma-separated output of the resolved x64 URL and an x86 URL derived by replacing x64x86; removed per-arch loop, SkipHttpErrorCheck usage, and PSVersion branching for redirection handling.

Sequence Diagram(s)

sequenceDiagram
  participant S as checkver script
  participant H as HTTP server
  Note over S: New single-call flow using System.Net.HttpWebRequest
  S->>H: GET (via HttpWebRequest)
  alt Server redirects
    H-->>S: 3xx -> final URL
    Note right of S: HttpWebRequest.ResponseUri holds final absolute URI
  else Direct OK
    H-->>S: 200 OK (ResponseUri = request URI)
  end
  S->>S: read ResponseUri (x64 URI)
  S->>S: derive x86 URI by replacing "x64" with "x86"
  S-->>Caller: return "x64_uri,x86_uri"
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A rabbit nudges one last hop,
Follows the redirect, never stops.
ResponseUri shows the door,
Swap x64 for x86, and fetch once more.
Two URLs found — I stash and hop! 🐇📦

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed The PR implements the primary coding requirement from issue [#16337] by rewriting the checkver logic in both foxit-pdf-reader.json and foxit-reader.json to correctly retrieve the latest version from the Foxit backend.
Out of Scope Changes Check ✅ Passed All modifications are confined to updating the checkver logic in the targeted Foxit Reader manifests, with no unrelated or out-of-scope changes introduced.
Description Check ✅ Passed The description includes a closing reference to issue #16337 and uses the required template checkboxes, with both items marked complete, thereby conforming to the repository’s PR description template.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title succinctly conveys that the PR fixes the checkver logic for the foxit(-pdf)-reader manifests at version 2025.2.1 and follows the conventional <manifest[@version]>: summary format.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddf7c62 and 00f9886.

📒 Files selected for processing (2)
  • bucket/foxit-pdf-reader.json (1 hunks)
  • bucket/foxit-reader.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: PowerShell
  • GitHub Check: WindowsPowerShell
  • GitHub Check: PullRequestHandler

@github-actions
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

foxit-pdf-reader

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

foxit-reader

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00f9886 and 1117a39.

📒 Files selected for processing (2)
  • bucket/foxit-pdf-reader.json (1 hunks)
  • bucket/foxit-reader.json (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16341
File: bucket/foxit-pdf-reader.json:47-50
Timestamp: 2025-10-15T11:54:31.297Z
Learning: In bucket/foxit-pdf-reader.json and bucket/foxit-reader.json, the checkver script uses MaximumRedirection 1 (not 0) for Invoke-WebRequest to properly handle Foxit's backend redirect behavior when parsing the Location header for version detection.
📚 Learning: 2025-10-15T11:54:31.297Z
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16341
File: bucket/foxit-pdf-reader.json:47-50
Timestamp: 2025-10-15T11:54:31.297Z
Learning: In bucket/foxit-pdf-reader.json and bucket/foxit-reader.json, the checkver script uses MaximumRedirection 1 (not 0) for Invoke-WebRequest to properly handle Foxit's backend redirect behavior when parsing the Location header for version detection.

Applied to files:

  • bucket/foxit-pdf-reader.json
  • bucket/foxit-reader.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: WindowsPowerShell

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 15, 2025

/verify

@github-actions
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

foxit-pdf-reader

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

foxit-reader

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

Copy link
Member

@z-Fng z-Fng left a comment

Choose a reason for hiding this comment

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

Thanks!

@z-Fng z-Fng changed the title [email protected]: Fix checkver logic after backend changes foxit(-pdf)[email protected]: Fix checkver Oct 15, 2025
@z-Fng z-Fng merged commit 1897187 into ScoopInstaller:master Oct 15, 2025
3 checks passed
@o-l-a-v o-l-a-v deleted the patch-1 branch October 15, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Foxit Reader checkver logic broke

2 participants