Repository navigation
fix: normalize site URLs + fail-fast on 404 NotFound (Stellantis batch) - #51
Merged
Merged
Conversation
Reported by Stellantis (Denis): site lists built by copy-pasting from a browser/OneDrive carry sharing-link query strings (?xsdata=...&sdata=...&ovuser=...). Passing such a URL to Get-PnPSiteVersionPolicy returns 404 NotFound, and the generic retry then retried it 5x with exponential backoff (~5 min per site) — crippling on a 3957-site batch. - Add Get-NormalizedSiteUrls: strip query strings/fragments, trailing slashes and whitespace, and de-duplicate. Applied to explicit SiteUrls before the sign-in anchor is chosen, and defensively to the SiteScope:All enumeration. - Add Test-IsNotFoundError and fail fast on 404 in Invoke-RetryCommand (structural error — a missing site cannot be conjured by retrying). Re-throw NotFound from the drift read and the Legacy/apply/batch-delete inner catches so it reaches the single per-site handler. - New per-site NotFound outcome: skipped (not failed), with report badge/KPI card, end-of-run count and advisory pointing at the canonical URL shape. Docs updated (CHANGELOG/RELEASE-NOTES under [Unreleased]; wiki resilience, outcome list and a NotFound troubleshooting entry). Tests: 165 pass.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Not-found classification can mislabel non-site failures, and normalization can silently leave no sites to process.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Normalizes SharePoint site URLs and avoids retry storms for missing sites.
Changes:
- Adds URL normalization and de-duplication.
- Introduces fail-fast 404 handling and
NotFoundreporting. - Adds tests and documentation for the new behavior.
| File | Description |
|---|---|
scripts/SPSCleanVersions.ps1 |
Implements normalization and NotFound handling. |
tests/SPSCleanVersions.Tests.ps1 |
Tests normalization, classification, and reporting. |
wiki/Usage.md |
Documents missing-site behavior. |
wiki/Home.md |
Updates resilience overview. |
wiki/Configuration.md |
Adds configuration and troubleshooting guidance. |
RELEASE-NOTES.md |
Records unreleased changes. |
CHANGELOG.md |
Documents user-visible behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ation, library-level NotFound
- Test-IsNotFoundError: drop the bare 'not found' alternative — it mis-classified unrelated
failures (missing list/column/certificate) as a missing site. Only explicit HTTP-status
evidence counts now ('status code is NotFound' / 404); '(404) Not Found' stays covered.
- Selected scope: fail configuration validation when normalization leaves no usable URL
(e.g. SiteUrls = ['', ' ']), instead of silently reporting a successful zero-site run.
- Legacy Set-PnPList catch: a NotFound identifies the targeted library (e.g. deleted between
enumeration and update), not the site — record it as a library-level Failed and continue,
rather than re-throwing and marking the whole site NotFound. Access-denied still bubbles up
(it implies no rights on the whole site). Site-level NotFound is still caught by Get-PnPList.
Tests: 167 pass.
…r-URL Copilot review: the previous wording implied a list composed entirely of sharing-link URLs could still 404, but normalization strips each URL's query string independently, so list composition is irrelevant. The real residual case is a copied link whose PATH is not a site-collection URL (e.g. /:f:/s/..., /personal/..., or a deep document path) — describe that instead.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reported by Stellantis (Denis) on a 3957-site batch. Two stacked problems:
carried a sharing-link query string (
?xsdata=...&sdata=...&ovuser=...). Passing such a URL toGet-PnPSiteVersionPolicyreturns 404 NotFound.generic backoff and was retried 5× (10+20+40+80+160 s ≈ 5 min per site) — crippling at
tenant scale.
Changes
Get-NormalizedSiteUrls— strips query strings/fragments, trailing slashes and whitespace,and de-duplicates. Applied to the explicit
SiteUrls(before the sign-in anchor is chosen) and,defensively, to the
SiteScope: Allenumeration.Test-IsNotFoundError+ fail-fast inInvoke-RetryCommand— a missing site cannot beconjured by retrying, so 404 fails fast instead of burning the backoff budget.
NotFoundoutcome — skipped (not failed), with report badge / KPI card,end-of-run count and an advisory pointing at the canonical URL shape. Re-thrown from the drift
read and the Legacy / apply / batch-delete inner catches so it reaches the single per-site handler.
[Unreleased]; wiki resilience section, outcome list and aNotFoundtroubleshooting entry.Testing
structural guards).
empty) → 4 clean canonical URLs.
Notes
[Unreleased], per RELEASING.md). A3.2.1patch release will becut after merge.
workflow requested by Denis, to be designed as its own security-sensitive chantier.