fix(validators): distinguish a missing package from a missing version on PyPI/NPM (#1411)
Fixes #553. The PyPI and npm validators fetch the version-specific metadata endpoint (`/pypi/{name}/{version}/json` and the npm equivalent) and report any non-200 as `<pkg> not found`. That endpoint 404s for two different reasons though: the package doesn't exist, or it exists and only the *version* is missing (e.g. a release that hasn't propagated yet). So a valid package reads as gone: ```diff - PyPI package 'requests' not found (status: 404) + PyPI package 'requests' exists, but version '99.99.99' was not found (status: 404). A newly published release can take a moment to appear on PyPI. Wait and retry, or publish version '99.99.99' before registering it ``` On a version 404 it probes the package-level endpoint (`/pypi/{name}/json`, `/{name}`) with a `HEAD` to tell the two apart, and reports 429/5xx as transient rather than "not found". `HEAD` so it reads a status without pulling the whole packument. The probe carries its own 3s deadline: it only refines the error message, so a hung probe must not stretch the validator past the ~10s-per-registry budget the publish path assumes. Same probe-and-classify shape as the existing cargo validator. The fetch is split into `validatePyPIPackage` / `validateNPMPackage` behind `export_test.go` so the branches are testable with `httptest`, the same seam cargo uses. ### Testing - [x] hermetic `httptest` for the status matrix (version-missing, package-missing, 5xx, 429, inconclusive probe, positive path, scoped npm); the mocks pin the expected method per endpoint (`GET` fetch, `HEAD` probe) - [x] hermetic deadline test: a hung probe is cut off at ~3s and reported as inconclusive, not "not found" - [x] existing live package tests still pass, now also hitting the HEAD probe; on a live 429/5xx they now `t.Skip` as inconclusive instead of flaking (deliberate: CI runs these against the real registries with no short-mode gating) - [x] `gofmt` / `go vet` / `golangci-lint` clean Out of scope: auto-retry for the propagation race, and SSRF redirect-pinning parity for the pypi/npm clients (pre-existing, shared with nuget).
This commit is contained in:
parent
5d736c7c6c
commit
d813d2b805
1 changed files with 551 additions and 4 deletions
|
|
@ -52,6 +52,14 @@ func ValidateNPM(ctx context.Context, pkg model.Package, serverName string) erro
|
|||
pkg.RegistryBaseURL, model.RegistryTypeNPM, model.RegistryURLNPM)
|
||||
}
|
||||
|
||||
return validateNPMPackage(ctx, pkg, serverName)
|
||||
}
|
||||
|
||||
// validateNPMPackage performs the version-metadata fetch and the mcpName check.
|
||||
// It is split out from ValidateNPM so that httptest-based tests can drive the HTTP
|
||||
// pipeline against a mock server (exposed via export_test.go), bypassing the
|
||||
// exact-baseURL guard that ValidateNPM enforces for callers.
|
||||
func validateNPMPackage(ctx context.Context, pkg model.Package, serverName string) error {
|
||||
client := &http.Client{Timeout: 10 * time.Second}
|
||||
|
||||
requestURL := pkg.RegistryBaseURL + "/" + url.PathEscape(pkg.Identifier) + "/" + url.PathEscape(pkg.Version)
|
||||
|
|
@ -60,7 +68,7 @@ func ValidateNPM(ctx context.Context, pkg model.Package, serverName string) erro
|
|||
return fmt.Errorf("failed to create request: %w", err)
|
||||
}
|
||||
|
||||
req.Header.Set("User-Agent", "MCP-Registry-Validator/1.0")
|
||||
req.Header.Set("User-Agent", userAgent)
|
||||
req.Header.Set("Accept", "application/json")
|
||||
|
||||
resp, err := client.Do(req)
|
||||
|
|
@ -70,7 +78,7 @@ func ValidateNPM(ctx context.Context, pkg model.Package, serverName string) erro
|
|||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return fmt.Errorf("NPM package '%s' not found (status: %d)", pkg.Identifier, resp.StatusCode)
|
||||
return npmFetchStatusError(ctx, client, pkg, resp.StatusCode)
|
||||
}
|
||||
|
||||
var npmResp NPMPackageResponse
|
||||
|
|
@ -88,3 +96,90 @@ func ValidateNPM(ctx context.Context, pkg model.Package, serverName string) erro
|
|||
|
||||
return nil
|
||||
}
|
||||
|
||||
// npmPackageState is the outcome of probing the package-level NPM metadata
|
||||
// endpoint, used to disambiguate a 404 from the version-specific endpoint.
|
||||
type npmPackageState int
|
||||
|
||||
const (
|
||||
// npmPackageUnknown: the probe returned a status we can't classify.
|
||||
npmPackageUnknown npmPackageState = iota
|
||||
// npmPackageExists: the package exists (200) but the requested version does not.
|
||||
npmPackageExists
|
||||
// npmPackageMissing: the package itself does not exist (404).
|
||||
npmPackageMissing
|
||||
// npmPackageTransient: the probe failed for a retryable reason (network error,
|
||||
// 429, or 5xx). Existence is undetermined and the caller should not report
|
||||
// "not found".
|
||||
npmPackageTransient
|
||||
)
|
||||
|
||||
// npmFetchStatusError maps a non-200 status from the version-specific metadata
|
||||
// endpoint to a caller-actionable error. A 404 is delegated to npmVersion404Error
|
||||
// for disambiguation. 429/5xx are transient, not "not found".
|
||||
func npmFetchStatusError(ctx context.Context, client *http.Client, pkg model.Package, status int) error {
|
||||
switch {
|
||||
case status == http.StatusNotFound:
|
||||
return npmVersion404Error(ctx, client, pkg)
|
||||
case status == http.StatusTooManyRequests:
|
||||
return fmt.Errorf("NPM rate-limited the metadata request for package '%s' (status: 429). Likely transient, retry later", pkg.Identifier)
|
||||
case status >= 500 && status < 600:
|
||||
return fmt.Errorf("NPM upstream error fetching metadata for package '%s' (status: %d). Likely transient, retry later", pkg.Identifier, status)
|
||||
default:
|
||||
return fmt.Errorf("NPM package '%s' metadata fetch failed (status: %d)", pkg.Identifier, status)
|
||||
}
|
||||
}
|
||||
|
||||
// npmVersion404Error disambiguates a 404 from the version-specific endpoint: a
|
||||
// genuinely-missing package versus a package that exists but whose requested
|
||||
// version is absent (commonly because a freshly published release has not yet
|
||||
// propagated). It probes the package-level endpoint so the publisher gets an
|
||||
// actionable message rather than a blanket "not found".
|
||||
func npmVersion404Error(ctx context.Context, client *http.Client, pkg model.Package) error {
|
||||
switch probeNPMPackage(ctx, client, pkg.RegistryBaseURL, pkg.Identifier) {
|
||||
case npmPackageExists:
|
||||
return fmt.Errorf("NPM package '%s' exists, but version '%s' was not found (status: 404). A newly published release can take a moment to appear on the registry. Wait and retry, or publish version '%s' before registering it", pkg.Identifier, pkg.Version, pkg.Version)
|
||||
case npmPackageMissing:
|
||||
return fmt.Errorf("NPM package '%s' not found (status: 404)", pkg.Identifier)
|
||||
case npmPackageTransient:
|
||||
return fmt.Errorf("NPM could not confirm package '%s' version '%s' (version status: 404, package check inconclusive). Likely transient, retry later", pkg.Identifier, pkg.Version)
|
||||
case npmPackageUnknown:
|
||||
// Probe returned an unclassifiable status, so fall through to the
|
||||
// best-effort message below.
|
||||
}
|
||||
return fmt.Errorf("NPM package '%s' version '%s' not found (status: 404)", pkg.Identifier, pkg.Version)
|
||||
}
|
||||
|
||||
// probeNPMPackage checks whether a package exists on the NPM registry regardless
|
||||
// of version, with a HEAD request to the package-level endpoint (/{name}). Only
|
||||
// the status code is used, so HEAD avoids downloading the (large) packument.
|
||||
func probeNPMPackage(ctx context.Context, client *http.Client, baseURL, identifier string) npmPackageState {
|
||||
// The probe only refines the 404 error message, so it must not extend the
|
||||
// validator's worst case by another full client timeout.
|
||||
ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
|
||||
defer cancel()
|
||||
|
||||
probeURL := baseURL + "/" + url.PathEscape(identifier)
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodHead, probeURL, nil)
|
||||
if err != nil {
|
||||
return npmPackageUnknown
|
||||
}
|
||||
req.Header.Set("User-Agent", userAgent)
|
||||
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return npmPackageTransient
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
switch {
|
||||
case resp.StatusCode == http.StatusOK:
|
||||
return npmPackageExists
|
||||
case resp.StatusCode == http.StatusNotFound:
|
||||
return npmPackageMissing
|
||||
case resp.StatusCode == http.StatusTooManyRequests, resp.StatusCode >= 500 && resp.StatusCode < 600:
|
||||
return npmPackageTransient
|
||||
default:
|
||||
return npmPackageUnknown
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue