From 3efe7bfa62ef22ff35e321096048cd4567db484c Mon Sep 17 00:00:00 2001 From: Matthias Dittrich Date: Mon, 19 Jun 2017 23:38:43 +0200 Subject: [PATCH 1/2] fix the warning by implementing internal 404 propagation and handling. There is still room for improvement: For example there should be no point in requesting additional urls when one already told you that the package is not available. This might not be true for custom servers on all urls but it must be true for the one nuget.exe is using (otherwise it wouldn't work and the server would be useless/only work with paket). --- .../Paket.IntegrationTests/PackSpecs.fs | 6 + src/Paket.Core/Common/Async.fs | 5 +- src/Paket.Core/Common/Utils.fs | 47 +++++-- src/Paket.Core/Dependencies/NuGet.fs | 97 ++++++------- src/Paket.Core/Dependencies/NuGetCache.fs | 32 +++-- src/Paket.Core/Dependencies/NuGetLocal.fs | 3 +- src/Paket.Core/Dependencies/NuGetV2.fs | 129 +++++++++++------- src/Paket.Core/Dependencies/NuGetV3.fs | 33 +++-- src/Paket.Core/Dependencies/RemoteDownload.fs | 23 ++-- src/Paket.Core/Versioning/PackageSources.fs | 6 +- .../Paket.Tests/NuGetOData/EmptyFeedList.xml | 10 ++ tests/Paket.Tests/NuGetOData/ODataSpecs.fs | 44 +++--- tests/Paket.Tests/Paket.Tests.fsproj | 3 + 13 files changed, 270 insertions(+), 168 deletions(-) create mode 100644 tests/Paket.Tests/NuGetOData/EmptyFeedList.xml diff --git a/integrationtests/Paket.IntegrationTests/PackSpecs.fs b/integrationtests/Paket.IntegrationTests/PackSpecs.fs index 12fe21be3f..c1814ee60c 100644 --- a/integrationtests/Paket.IntegrationTests/PackSpecs.fs +++ b/integrationtests/Paket.IntegrationTests/PackSpecs.fs @@ -10,6 +10,7 @@ open System.Diagnostics open System.IO.Compression open Paket.Domain open Paket +open Paket.NuGetCache let getDependencies = Paket.NuGet.NuGetPackageCache.getDependencies @@ -97,6 +98,7 @@ let ``#1429 pack deps from template``() = let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldNotContain (PackageName "PaketBug2") // it's not packed in same round @@ -113,6 +115,7 @@ let ``#1429 pack deps``() = let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "PaketBug2") @@ -129,6 +132,7 @@ let ``#1429 pack deps using minimum-from-lock-file``() = let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") let packageName, versionRequirement, restrictions = details |> getDependencies |> List.filter (fun (x,_,_) -> x = PackageName "MySql.Data") |> List.head @@ -145,6 +149,7 @@ let ``#1429 pack deps without minimum-from-lock-file uses dependencies file rang let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") let packageName, versionRequirement, restrictions = details |> getDependencies |> List.filter (fun (x,_,_) -> x = PackageName "MySql.Data") |> List.head @@ -177,6 +182,7 @@ let ``#1429 pack deps with minimum-from-lock-file uses specifc dependencies file let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") let packageName, versionRequirement, restrictions = details |> getDependencies |> List.filter (fun (x,_,_) -> x = PackageName "MySql.Data") |> List.head diff --git a/src/Paket.Core/Common/Async.fs b/src/Paket.Core/Common/Async.fs index 8b24968855..417be9cb71 100644 --- a/src/Paket.Core/Common/Async.fs +++ b/src/Paket.Core/Common/Async.fs @@ -55,7 +55,10 @@ module AsyncExtensions = if t.IsCanceled then cancel (OperationCanceledException("The underlying task has been cancelled")) elif t.IsFaulted then - error t.Exception + if t.Exception.InnerExceptions.Count = 1 then + error t.Exception.InnerExceptions.[0] + else + error t.Exception else success t.Result)) |> ignore } |> fun a -> Async.Start(a, ct) diff --git a/src/Paket.Core/Common/Utils.fs b/src/Paket.Core/Common/Utils.fs index cccfbdfa14..c3505d028e 100644 --- a/src/Paket.Core/Common/Utils.fs +++ b/src/Paket.Core/Common/Utils.fs @@ -545,6 +545,7 @@ let createWebClient (url,auth:Auth option) = open System.Diagnostics open System.Threading open System.Collections.Generic +open System.Runtime.ExceptionServices /// [omit] let downloadFromUrl (auth:Auth option, url : string) (filePath: string) = @@ -580,7 +581,7 @@ let getFromUrl (auth:Auth option, url : string, contentType : string) = } let getXmlFromUrl (auth:Auth option, url : string) = - async { + async { try use client = createWebClient (url,auth) // mimic the headers sent from nuget client to odata/ endpoints @@ -593,17 +594,36 @@ let getXmlFromUrl (auth:Auth option, url : string) = use _ = Profile.startCategory Profile.Category.NuGetRequest return! client.DownloadStringTaskAsync (Uri url) |> Async.awaitTaskWithToken (fun () -> failwithf "Uri '%O' failed to respond and cancellation was requested." url) with - | exn -> + | exn -> return raise <| Exception(sprintf "Could not retrieve data from '%s'" url, exn) } - + +type SafeWebResult<'a> = + | NotFound + | SuccessResponse of 'a + | UnknownError of ExceptionDispatchInfo +module SafeWebResult = + let map f s = + match s with + | SuccessResponse r -> SuccessResponse (f r) + | UnknownError err -> UnknownError err + | NotFound -> NotFound + let asResult s = + match s with + | NotFound -> + let notFound = Exception("Request returned 404") + ExceptionDispatchInfo.Capture notFound + |> FSharp.Core.Result.Error + | UnknownError err -> FSharp.Core.Result.Error err + | SuccessResponse s -> FSharp.Core.Result.Ok s + /// [omit] let safeGetFromUrl (auth:Auth option, url : string, contentType : string) = - async { - try + async { + try let uri = Uri url use client = createWebClient (url,auth) - + if notNullOrEmpty contentType then addAcceptHeader client contentType #if NETSTANDARD1_6 @@ -614,12 +634,15 @@ let safeGetFromUrl (auth:Auth option, url : string, contentType : string) = verbosefn "Starting request to '%O'" uri use _ = Profile.startCategory Profile.Category.NuGetRequest let! raw = client.DownloadStringTaskAsync(uri) |> Async.awaitTaskWithToken (fun () -> failwithf "Uri '%O' failed to respond and cancellation was requested." uri) - return FSharp.Core.Result.Ok raw - with e -> - if verbose then - Logging.verbosefn "Error while retrieving '%s': %O" url e - let cap = System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture e - return FSharp.Core.Result.Error cap + return SuccessResponse raw + with + | :? WebException as w -> + match w.Response with + | :? HttpWebResponse as wr when wr.StatusCode = HttpStatusCode.NotFound -> return NotFound + | _ -> + if verbose then + Logging.verbosefn "Error while retrieving '%s': %O" url w + return UnknownError (ExceptionDispatchInfo.Capture w) } let mutable autoAnswer = None diff --git a/src/Paket.Core/Dependencies/NuGet.fs b/src/Paket.Core/Dependencies/NuGet.fs index f36d6a6a1a..ea9f5cb293 100644 --- a/src/Paket.Core/Dependencies/NuGet.fs +++ b/src/Paket.Core/Dependencies/NuGet.fs @@ -14,6 +14,7 @@ open System.Net open System.Runtime.ExceptionServices open System.Text open FSharp.Polyfill +open Paket.NuGetCache let DownloadLicense(root,force,packageName:PackageName,version:SemVerInfo,licenseUrl,targetFileName) = @@ -120,69 +121,63 @@ let tryNuGetV3 (auth, nugetV3Url, package:PackageName) = let rec private getPackageDetails alternativeProjectRoot root force (sources:PackageSource list) packageName (version:SemVerInfo) : Async = async { - let tryV2 source (nugetSource:NugetSource) force = async { - let! result = - NuGetV2.getDetailsFromNuGet - force - (nugetSource.Authentication |> Option.map toBasicAuth) - nugetSource.Url - packageName - version - return Choice1Of2(source,result) } - - let tryV3 source nugetSource force = async { + let tryV2 (nugetSource:NugetSource) force = + NuGetV2.getDetailsFromNuGet + force + (nugetSource.Authentication |> Option.map toBasicAuth) + nugetSource.Url + packageName + version + + let tryV3 (nugetSource:NugetV3Source) force = if nugetSource.Url.Contains("myget.org") || nugetSource.Url.Contains("nuget.org") || nugetSource.Url.Contains("visualstudio.com") || nugetSource.Url.Contains("/nuget/v3/") then match NuGetV3.calculateNuGet2Path nugetSource.Url with | Some url -> - let! result = - NuGetV2.getDetailsFromNuGet - force - (nugetSource.Authentication |> Option.map toBasicAuth) - url - packageName - version - return Choice1Of2(source,result) + NuGetV2.getDetailsFromNuGet + force + (nugetSource.Authentication |> Option.map toBasicAuth) + url + packageName + version | _ -> - let! result = NuGetV3.GetPackageDetails force nugetSource packageName version - return Choice1Of2(source,result) + NuGetV3.GetPackageDetails force nugetSource packageName version else - let! result = NuGetV3.GetPackageDetails force nugetSource packageName version - return Choice1Of2(source,result) } + NuGetV3.GetPackageDetails force nugetSource packageName version let getPackageDetails force = // helper to work through the list sequentially - let rec trySelectFirst errors workLeft = + let rec trySelectFirst workLeft = async { match workLeft with - | work :: rest -> + | (source, work) :: rest -> let! r = work match r with - | Choice1Of2 result -> return Choice1Of2 result - | Choice2Of2 error -> return! trySelectFirst (error::errors) rest - | [] -> return Choice2Of2 errors + | ODataSearchResult.Match result -> return Some (source, result) + | ODataSearchResult.EmptyResult -> return! trySelectFirst rest + | [] -> return None } sources |> List.sortBy (fun source -> match source with // put local caches to the end | LocalNuGet(_,Some _) -> true | _ -> false) - |> List.map (fun source -> async { + |> List.map (fun source -> source, async { try match source with | NuGetV2 nugetSource -> - return! tryV2 source nugetSource force + return! tryV2 nugetSource force | NuGetV3 nugetSource when NuGetV2.urlSimilarToTfsOrVsts nugetSource.Url -> match NuGetV3.calculateNuGet2Path nugetSource.Url with | Some url -> let nugetSource : NugetSource = { Url = url Authentication = nugetSource.Authentication } - return! tryV2 source nugetSource force + return! tryV2 nugetSource force | _ -> - return! tryV3 source nugetSource force + return! tryV3 nugetSource force | NuGetV3 nugetSource -> try - return! tryV3 source nugetSource force + return! tryV3 nugetSource force with | exn -> match NuGetV3.calculateNuGet2Path nugetSource.Url with @@ -190,23 +185,18 @@ let rec private getPackageDetails alternativeProjectRoot root force (sources:Pac let nugetSource : NugetSource = { Url = url Authentication = nugetSource.Authentication } - return! tryV2 source nugetSource force + return! tryV2 nugetSource force | _ -> raise exn - return! tryV3 source nugetSource force - - | LocalNuGet(path,Some _) -> - let! result = NuGetLocal.getDetailsFromLocalNuGetPackage true alternativeProjectRoot root path packageName version - return Choice1Of2(source,result) - | LocalNuGet(path,None) -> - let! result = NuGetLocal.getDetailsFromLocalNuGetPackage false alternativeProjectRoot root path packageName version - return Choice1Of2(source,result) + return! tryV3 nugetSource force + + | LocalNuGet(path,hasCache) -> + return! NuGetLocal.getDetailsFromLocalNuGetPackage hasCache.IsSome alternativeProjectRoot root path packageName version with e -> - if verbose then - verbosefn "Source '%O' exception: %O" source e - let capture = ExceptionDispatchInfo.Capture e - return Choice2Of2 capture }) - |> trySelectFirst [] + traceWarnfn "Source '%O' exception: %O" source e + //let capture = ExceptionDispatchInfo.Capture e + return EmptyResult }) + |> trySelectFirst let! maybePackageDetails = getPackageDetails force let! source,nugetObject = @@ -219,17 +209,10 @@ let rec private getPackageDetails alternativeProjectRoot root force (sources:Pac failwithf "Couldn't get package details for package %O %O, because no sources were specified." packageName version | sources -> failwithf "Couldn't get package details for package %O %O on any of %A." packageName version sources - + match maybePackageDetails with - | Choice2Of2 ([]) -> return fallback() - | Choice2Of2 (h::restError) -> - for error in restError do - if not verbose then - // Otherwise the error was already mentioned above - traceWarnfn "Ignoring: %s" error.SourceException.Message - h.Throw() - return fallback() - | Choice1Of2 packageDetails -> return packageDetails + | None -> return fallback() + | Some packageDetails -> return packageDetails } let encodeURL (url:string) = diff --git a/src/Paket.Core/Dependencies/NuGetCache.fs b/src/Paket.Core/Dependencies/NuGetCache.fs index d2ca1b8e3c..f6c3e0c577 100644 --- a/src/Paket.Core/Dependencies/NuGetCache.fs +++ b/src/Paket.Core/Dependencies/NuGetCache.fs @@ -49,7 +49,7 @@ type NuGetResponseGetVersions = | SuccessVersionResponse _ -> true | ProtocolNotCached -> false | FailedVersionRequest _ -> false -type NuGetResponseGetVersionsSimple = FSharp.Core.Result +type NuGetResponseGetVersionsSimple = SafeWebResult type NuGetRequestGetVersions = { DoRequest : unit -> Async Url : string } @@ -61,8 +61,9 @@ type NuGetRequestGetVersions = let! res = f() return match res with - | FSharp.Core.Result.Ok r -> SuccessVersionResponse r - | FSharp.Core.Result.Error err -> FailedVersionRequest { Url = url; Error = err } + | SuccessResponse r -> SuccessVersionResponse r + | NotFound -> SuccessVersionResponse [||] + | UnknownError err -> FailedVersionRequest { Url = url; Error = err } }) static member run (r:NuGetRequestGetVersions) : Async = async { @@ -117,7 +118,7 @@ type NuGetPackageCache = Version: string CacheVersion: string } - static member CurrentCacheVersion = "5.0" + static member CurrentCacheVersion = "5.1" // TODO: is there a better way? for now we use static member because that works with type abbreviations... //module NuGetPackageCache = @@ -156,13 +157,27 @@ let getCacheFiles cacheVersion nugetURL (packageName:PackageName) (version:SemVe |> Seq.filter (fun p -> Path.GetFileName p <> packageUrl) |> Seq.toList FileInfo(newFile), oldFiles -let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemVerInfo) (get : unit -> NuGetPackageCache Async) : NuGetPackageCache Async = + +type ODataSearchResult = + | EmptyResult + | Match of NuGetPackageCache +module ODataSearchResult = + let get x = + match x with + | EmptyResult -> failwithf "Cannot call get on 'EmptyResult'" + | Match r -> r +let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemVerInfo) (get : unit -> ODataSearchResult Async) : ODataSearchResult Async = let cacheFile, oldFiles = getCacheFiles NuGetPackageCache.CurrentCacheVersion nugetURL packageName version oldFiles |> Seq.iter (fun f -> File.Delete f) - let get() = + let get() = async { let! result = get() - File.WriteAllText(cacheFile.FullName,JsonConvert.SerializeObject(result)) + match result with + | ODataSearchResult.Match result -> + File.WriteAllText(cacheFile.FullName,JsonConvert.SerializeObject(result)) + | _ -> + // TODO: Should we cache 404? Probably not. + () return result } async { @@ -171,7 +186,6 @@ let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemV let cacheResult = try let cachedObject = JsonConvert.DeserializeObject json - if (PackageName cachedObject.PackageName <> packageName) || (cachedObject.Version <> version.Normalize()) then @@ -189,7 +203,7 @@ let getDetailsFromCacheOr force nugetURL (packageName:PackageName) (version:SemV traceWarnfn "Error while loading cache: %s" exn.Message None match cacheResult with - | Some res -> return res + | Some res -> return ODataSearchResult.Match res | None -> return! get() else return! get() diff --git a/src/Paket.Core/Dependencies/NuGetLocal.fs b/src/Paket.Core/Dependencies/NuGetLocal.fs index b09b1d7809..b23049b3db 100644 --- a/src/Paket.Core/Dependencies/NuGetLocal.fs +++ b/src/Paket.Core/Dependencies/NuGetLocal.fs @@ -27,7 +27,7 @@ let getAllVersionsFromLocalPath (isCache, localNugetPath, package:PackageName, a let _match = Regex(sprintf @"^%O\.(\d.*)\.nupkg" package, RegexOptions.IgnoreCase).Match(fi.Name) if _match.Groups.Count > 1 then Some _match.Groups.[1].Value else None) |> Seq.toArray - return Result.Ok(versions) + return SuccessResponse(versions) }) /// Reads packageName and version from .nupkg file name @@ -109,4 +109,5 @@ let getDetailsFromLocalNuGetPackage isCache alternativeProjectRoot root localNuG Version = version.Normalize() Unlisted = isCache } |> NuGetPackageCache.withDependencies nuspec.Dependencies + |> ODataSearchResult.Match } \ No newline at end of file diff --git a/src/Paket.Core/Dependencies/NuGetV2.fs b/src/Paket.Core/Dependencies/NuGetV2.fs index 4414ad2802..ffb445b33b 100644 --- a/src/Paket.Core/Dependencies/NuGetV2.fs +++ b/src/Paket.Core/Dependencies/NuGetV2.fs @@ -87,10 +87,10 @@ let tryGetAllVersionsFromNugetODataWithFilter (auth, nugetURL, package:PackageNa async { try let! result = followODataLink auth url - return Result.Ok result + return SuccessResponse result with exn -> let cap = ExceptionDispatchInfo.Capture exn - return Result.Error cap + return UnknownError cap }) let tryGetAllVersionsFromNugetODataFindById (auth, nugetURL, package:PackageName) = @@ -99,10 +99,10 @@ let tryGetAllVersionsFromNugetODataFindById (auth, nugetURL, package:PackageName async { try let! result = followODataLink auth url - return Result.Ok result + return SuccessResponse result with exn -> let cap = ExceptionDispatchInfo.Capture exn - return Result.Error cap + return UnknownError cap }) let tryGetAllVersionsFromNugetODataFindByIdNewestFirst (auth, nugetURL, package:PackageName) = @@ -111,24 +111,21 @@ let tryGetAllVersionsFromNugetODataFindByIdNewestFirst (auth, nugetURL, package: async { try let! result = followODataLink auth url - return Result.Ok result + return SuccessResponse result with exn -> let cap = ExceptionDispatchInfo.Capture exn - return Result.Error cap + return UnknownError cap }) -let parseODataDetails(url,nugetURL,packageName:PackageName,version:SemVerInfo,raw) = +let private getXmlDoc url raw = let doc = XmlDocument() try doc.LoadXml raw with - | _ -> failwithf "Could not parse response from %s as OData.%sData:%s%s" url Environment.NewLine Environment.NewLine raw - - let entry = - match (doc |> getNode "feed" |> optGetNode "entry" ) ++ (doc |> getNode "entry") with - | Some node -> node - | _ -> failwithf "unable to find entry node for package %O %O in %s" packageName version raw + | e -> raise <| Exception(sprintf "Could not parse response from %s as OData.%sData:%s%s" url Environment.NewLine Environment.NewLine raw, e) + doc +let private handleODataEntry nugetURL packageName version entry = let officialName = match (entry |> getNode "properties" |> optGetNode "Id") ++ (entry |> getNode "title") with | Some node -> node.InnerText @@ -201,8 +198,45 @@ let parseODataDetails(url,nugetURL,packageName:PackageName,version:SemVerInfo,ra Unlisted = publishDate = Constants.MagicUnlistingDate } |> NuGetPackageCache.withDependencies dependencies + + +// parse search results. +let parseODataListDetails (url,nugetURL,packageName:PackageName,version:SemVerInfo,raw) : ODataSearchResult = + let doc = getXmlDoc url raw + + let feedNode = + match doc |> getNode "feed" with + | Some node -> node + | None -> failwithf "Could not find 'entry' node for package %O %O" packageName version + match feedNode |> getNodes "entry" with + | [] -> + // When no entry node is found our search did not yield anything. + EmptyResult + | entry :: t -> + if t.Length > 0 then + traceWarnfn "Got multiple results in OData query ('%s') for package %O %O, the feed might be broken" url packageName version + handleODataEntry nugetURL packageName version entry + |> ODataSearchResult.Match + +let parseODataEntryDetails (url,nugetURL,packageName:PackageName,version:SemVerInfo,raw) = + let doc = getXmlDoc url raw + match doc |> getNode "entry" with + | Some entry -> + handleODataEntry nugetURL packageName version entry + | None -> failwithf "Could not find 'entry' node for package %O %O" packageName version + let getDetailsFromNuGetViaODataFast auth nugetURL (packageName:PackageName) (version:SemVerInfo) = async { + let fallback () = + async { + let url = sprintf "%s/Packages?$filter=(tolower(Id) eq '%s') and (Version eq '%O')" nugetURL (packageName.CompareString) version + let! raw = getFromUrl(auth,url,acceptXml) + if verbose then + tracefn "Response from %s:" url + tracefn "" + tracefn "%s" raw + return parseODataListDetails(url,nugetURL,packageName,version,raw) + } let firstUrl = sprintf "%s/Packages?$filter=(tolower(Id) eq '%s') and (NormalizedVersion eq '%s')" nugetURL (packageName.CompareString) (version.Normalize()) try let! raw = getFromUrl(auth,firstUrl,acceptXml) @@ -210,16 +244,15 @@ let getDetailsFromNuGetViaODataFast auth nugetURL (packageName:PackageName) (ver tracefn "Response from %s:" firstUrl tracefn "" tracefn "%s" raw - return parseODataDetails(firstUrl,nugetURL,packageName,version,raw) + match parseODataListDetails(firstUrl,nugetURL,packageName,version,raw) with + | EmptyResult -> + if verbose then tracefn "No results, trying again with Version instead of NormalizedVersion." + return! fallback() + | res -> return res with ex -> - traceWarnfn "Failed to getDetailsFromNuGetViaODataFast '%s'. Trying with Version instead of NormalizedVersion: %O" firstUrl ex - let url = sprintf "%s/Packages?$filter=(tolower(Id) eq '%s') and (Version eq '%O')" nugetURL (packageName.CompareString) version - let! raw = getFromUrl(auth,url,acceptXml) - if verbose then - tracefn "Response from %s:" url - tracefn "" - tracefn "%s" raw - return parseODataDetails(url,nugetURL,packageName,version,raw) + // TODO: Remove this 'with' eventually when this warning is no longer reported + traceWarnfn "Failed to getDetailsFromNuGetViaODataFast '%s'. Trying with Version instead of NormalizedVersion (Please report this warning!): %O" firstUrl ex + return! fallback() } let urlSimilarToTfsOrVsts url = @@ -234,8 +267,9 @@ let getDetailsFromNuGetViaOData auth nugetURL (packageName:PackageName) (version let! raw = match response with - | FSharp.Core.Result.Ok r -> async { return r } - | FSharp.Core.Result.Error err when + | SafeWebResult.SuccessResponse r -> async { return Some r } + | SafeWebResult.NotFound -> async { return None } + | SafeWebResult.UnknownError err when String.containsIgnoreCase "myget.org" nugetURL || String.containsIgnoreCase "nuget.org" nugetURL || String.containsIgnoreCase "visualstudio.com" nugetURL -> @@ -243,40 +277,41 @@ let getDetailsFromNuGetViaOData auth nugetURL (packageName:PackageName) (version System.Exception( sprintf "Could not get package details for %O from %s" packageName nugetURL, err.SourceException) - | FSharp.Core.Result.Error err -> - try - let url = sprintf "%s/odata/Packages(Id='%O',Version='%O')" nugetURL packageName version - getXmlFromUrl(auth,url) - with e -> - raise <| System.AggregateException(err.SourceException, e) + | SafeWebResult.UnknownError err -> + traceWarnfn "Failed to find defails '%s' from '%s'. trying again with /odata/Packages. Please report this." err.SourceException.Message url + if verbose then + tracefn "Details of last error (%s): %O" url err.SourceException + async { + try + let url = sprintf "%s/odata/Packages(Id='%O',Version='%O')" nugetURL packageName version + let! raw = getXmlFromUrl(auth,url) + return Some raw + with e -> + return raise <| System.AggregateException(err.SourceException, e) + } if verbose then tracefn "Response from %s:" url tracefn "" - tracefn "%s" raw - return parseODataDetails(url,nugetURL,packageName,version,raw) } + tracefn "%s" (match raw with Some s -> s | _ -> "NOTFOUND 404") + match raw with + | Some raw -> + return parseODataEntryDetails(url,nugetURL,packageName,version,raw) |> ODataSearchResult.Match + | None -> return ODataSearchResult.EmptyResult } async { try - let! result = getDetailsFromNuGetViaODataFast auth nugetURL packageName version - if urlSimilarToTfsOrVsts nugetURL && result |> NuGetPackageCache.getDependencies |> List.isEmpty then + let! result = + // See https://github.com/fsprojects/Paket/issues/2213 // TODO: There is a bug in VSTS, so we can't trust this protocol. Remove when VSTS is fixed // TODO: TFS has the same bug - return! queryPackagesProtocol packageName - else - return result - with e -> + if urlSimilarToTfsOrVsts nugetURL then queryPackagesProtocol packageName + else getDetailsFromNuGetViaODataFast auth nugetURL packageName version + return result + with e when not (urlSimilarToTfsOrVsts nugetURL) -> traceWarnfn "Failed to get package details '%s'. This feeds implementation might be broken." e.Message if verbose then tracefn "Details: %O" e - try - return! queryPackagesProtocol packageName - with e -> - traceWarnfn "Failed to get package details (again) '%s'. This feeds implementation might be broken." e.Message - if verbose then tracefn "Details: %O" e - // try uppercase version as workaround for https://github.com/fsprojects/Paket/issues/2145 - Bad! - let name = packageName.ToString() - let uppercase = packageName.ToString().[0].ToString().ToUpper() + name.Substring(1) - return! queryPackagesProtocol (PackageName uppercase) + return! queryPackagesProtocol packageName } let getDetailsFromNuGet force auth nugetURL packageName version = diff --git a/src/Paket.Core/Dependencies/NuGetV3.fs b/src/Paket.Core/Dependencies/NuGetV3.fs index 6202ced2c6..d4703d2a4c 100644 --- a/src/Paket.Core/Dependencies/NuGetV3.fs +++ b/src/Paket.Core/Dependencies/NuGetV3.fs @@ -112,7 +112,7 @@ let internal findAutoCompleteVersionsForPackage(v3Url, auth, packageName:Domain. let! response = safeGetFromUrl(auth,url,acceptJson) // NuGet is showing old versions first return response - |> FSharp.Core.Result.map (fun text -> + |> SafeWebResult.map (fun text -> let versions = let extracted = extractAutoCompleteVersions text if extracted.Length > maxResults then @@ -137,7 +137,7 @@ let internal findVersionsForPackage(v3Url, auth, packageName:Domain.PackageName) let! response = safeGetFromUrl(auth,url,acceptJson) // NuGet is showing old versions first return response - |> Result.map (fun text -> + |> SafeWebResult.map (fun text -> let versions = extractVersions text SemVer.SortVersions versions) @@ -157,12 +157,12 @@ let private getPackages(auth, nugetURL, packageNamePrefix, maxResults) = async { | Some url -> let query = sprintf "%s?q=%s&take=%d" url packageNamePrefix maxResults let! response = safeGetFromUrl(auth |> Option.map toBasicAuth,query,acceptJson) - match response with - | FSharp.Core.Result.Ok text -> return FSharp.Core.Result.Ok (extractPackages text) - | FSharp.Core.Result.Error err -> return FSharp.Core.Result.Error err + match SafeWebResult.asResult response with + | Result.Ok text -> return Result.Ok (extractPackages text) + | Result.Error err -> return Result.Error err | None -> if verbose then tracefn "Could not calculate search api from %s" nugetURL - return FSharp.Core.Result.Ok [||] + return Result.Ok [||] } /// Uses the NuGet v3 autocomplete service to retrieve all packages with the given prefix. @@ -207,9 +207,10 @@ let getRegistration (source : NugetV3Source) (packageName:PackageName) (version: let! rawData = safeGetFromUrl (source.Authentication |> Option.map toBasicAuth, url, acceptJson) return match rawData with - | FSharp.Core.Result.Error err -> + | NotFound -> None //raise <| System.Exception(sprintf "could not get registration data (404) from '%s'" url) + | UnknownError err -> raise <| System.Exception(sprintf "could not get registration data from %s" url, err.SourceException) - | FSharp.Core.Result.Ok x -> JsonConvert.DeserializeObject(x) + | SuccessResponse x -> Some (JsonConvert.DeserializeObject(x)) } let getCatalog url auth = @@ -217,14 +218,19 @@ let getCatalog url auth = let! rawData = safeGetFromUrl (auth, url, acceptJson) return match rawData with - | FSharp.Core.Result.Error err -> + | NotFound -> + raise <| System.Exception(sprintf "could not get catalog data (404) from '%s'" url) + | UnknownError err -> raise <| System.Exception(sprintf "could not get catalog data from %s" url, err.SourceException) - | FSharp.Core.Result.Ok x -> JsonConvert.DeserializeObject(x) + | SuccessResponse x -> JsonConvert.DeserializeObject(x) } -let getPackageDetails (source:NugetV3Source) (packageName:PackageName) (version:SemVerInfo) : Async = +let getPackageDetails (source:NugetV3Source) (packageName:PackageName) (version:SemVerInfo) : Async = async { let! registrationData = getRegistration source packageName version + match registrationData with + | None -> return EmptyResult + | Some registrationData -> let! catalogData = getCatalog registrationData.CatalogEntry (source.Authentication |> Option.map toBasicAuth) let dependencies = @@ -264,6 +270,7 @@ let getPackageDetails (source:NugetV3Source) (packageName:PackageName) (version: Version = version.Normalize() CacheVersion = NuGetPackageCache.CurrentCacheVersion } |> NuGetPackageCache.withDependencies optimized + |> ODataSearchResult.Match } let loadFromCacheOrGetDetails (force:bool) @@ -280,7 +287,7 @@ let loadFromCacheOrGetDetails (force:bool) let! details = getPackageDetails source packageName version return true,details else - return false,cachedObject + return false,ODataSearchResult.Match cachedObject with _ -> let! details = getPackageDetails source packageName version return true,details @@ -290,7 +297,7 @@ let loadFromCacheOrGetDetails (force:bool) } /// Uses the NuGet v3 registration endpoint to retrieve package details . -let GetPackageDetails (force:bool) (source:NugetV3Source) (packageName:PackageName) (version:SemVerInfo) : Async = +let GetPackageDetails (force:bool) (source:NugetV3Source) (packageName:PackageName) (version:SemVerInfo) : Async = getDetailsFromCacheOr force source.Url diff --git a/src/Paket.Core/Dependencies/RemoteDownload.fs b/src/Paket.Core/Dependencies/RemoteDownload.fs index de85ef9d7d..cdfa7db5c5 100644 --- a/src/Paket.Core/Dependencies/RemoteDownload.fs +++ b/src/Paket.Core/Dependencies/RemoteDownload.fs @@ -31,24 +31,26 @@ let getSHA1OfBranch origin owner project (versionRestriction:VersionRestriction) let url = sprintf "https://api.github.com/repos/%s/%s/commits/%s" owner project branch let! document = lookupDocument(auth authKey url,url) match document with - | FSharp.Core.Result.Ok (document) -> + | SuccessResponse (document) -> let json = JObject.Parse(document) return json.["sha"].ToString() - | FSharp.Core.Result.Error (err) -> - return - raise <| new Exception(sprintf "Could not find hash for %s" url, err.SourceException) + | NotFound -> + return raise <| new Exception(sprintf "Could not find (404) hash for %s" url) + | UnknownError (err) -> + return raise <| new Exception(sprintf "Could not find hash for %s" url, err.SourceException) | ModuleResolver.Origin.GistLink -> let branch = ModuleResolver.getVersionRequirement versionRestriction let url = sprintf "https://api.github.com/gists/%s/%s" project branch let! document = lookupDocument(auth authKey url,url) match document with - | FSharp.Core.Result.Ok document -> + | SuccessResponse document -> let json = JObject.Parse(document) let latest = json.["history"].First.["version"] return latest.ToString() - | FSharp.Core.Result.Error err -> - return - raise <| new Exception(sprintf "Could not find hash for %s" url, err.SourceException) + | NotFound -> + return raise <| new Exception(sprintf "Could not find hash for %s" url) + | UnknownError err -> + return raise <| new Exception(sprintf "Could not find hash for %s" url, err.SourceException) | ModuleResolver.Origin.GitLink (LocalGitOrigin path) -> let path = path.Replace(@"file:///", "") let branch = @@ -136,12 +138,13 @@ let downloadDependenciesFile(force,rootPath,groupName,parserF,remoteFile:ModuleR let text,depsFile = match result with - | FSharp.Core.Result.Ok text -> + | SuccessResponse text -> try text,parserF text with | _ -> "",parserF "" - | FSharp.Core.Result.Error err -> + | NotFound + | UnknownError _ -> "",parserF "" Directory.CreateDirectory(destination.FullName |> Path.GetDirectoryName) |> ignore diff --git a/src/Paket.Core/Versioning/PackageSources.fs b/src/Paket.Core/Versioning/PackageSources.fs index 4899de366b..cad547fba9 100644 --- a/src/Paket.Core/Versioning/PackageSources.fs +++ b/src/Paket.Core/Versioning/PackageSources.fs @@ -97,9 +97,11 @@ let getNuGetV3Resource (source : NugetV3Source) (resourceType : NugetV3ResourceT let! rawData = safeGetFromUrl(basicAuth, source.Url, acceptJson) let rawData = match rawData with - | FSharp.Core.Result.Error e -> + | NotFound -> + raise <| new Exception(sprintf "Could not load resources (404) from '%s'" source.Url) + | UnknownError e -> raise <| new Exception(sprintf "Could not load resources from '%s'" source.Url, e.SourceException) - | FSharp.Core.Result.Ok x -> x + | SuccessResponse x -> x let json = JsonConvert.DeserializeObject(rawData) let resources = diff --git a/tests/Paket.Tests/NuGetOData/EmptyFeedList.xml b/tests/Paket.Tests/NuGetOData/EmptyFeedList.xml new file mode 100644 index 0000000000..66c7e5e4b3 --- /dev/null +++ b/tests/Paket.Tests/NuGetOData/EmptyFeedList.xml @@ -0,0 +1,10 @@ + + + http://schemas.datacontract.org/2004/07/ + + <updated>2017-06-19T18:52:18Z</updated> + <link rel="self" href="https://www.nuget.org/api/v2/Packages" /> + <author> + <name /> + </author> +</feed> \ No newline at end of file diff --git a/tests/Paket.Tests/NuGetOData/ODataSpecs.fs b/tests/Paket.Tests/NuGetOData/ODataSpecs.fs index 2ecbe2b5ba..b7f560d3bf 100644 --- a/tests/Paket.Tests/NuGetOData/ODataSpecs.fs +++ b/tests/Paket.Tests/NuGetOData/ODataSpecs.fs @@ -10,16 +10,20 @@ open Paket.NuGetV3 open Paket.Requirements open Paket.TestHelpers open Domain +open Paket.NuGetCache let fakeUrl = "http://doesntmatter" -let parse fileName = +let parseList fileName = System.Environment.CurrentDirectory <- Path.GetDirectoryName __SOURCE_DIRECTORY__ - parseODataDetails("tenp",fakeUrl,PackageName "package",SemVer.Parse "0",File.ReadAllText fileName) + parseODataListDetails("tenp",fakeUrl,PackageName "package",SemVer.Parse "0",File.ReadAllText fileName) +let parseEntry fileName = + System.Environment.CurrentDirectory <- Path.GetDirectoryName __SOURCE_DIRECTORY__ + parseODataEntryDetails("tenp",fakeUrl,PackageName "package",SemVer.Parse "0",File.ReadAllText fileName) [<Test>] let ``can detect explicit dependencies for Fantomas``() = - parse "NuGetOData/Fantomas.xml" + parseEntry "NuGetOData/Fantomas.xml" |> shouldEqual { PackageName = "Fantomas" DownloadUrl = "http://www.nuget.org/api/v2/package/Fantomas/1.6.0" @@ -30,11 +34,16 @@ let ``can detect explicit dependencies for Fantomas``() = Version = "1.6.0" SourceUrl = fakeUrl } +[<Test>] +let ``can parse an empty odata result``() = + parseList "NuGetOData/EmptyFeedList.xml" + |> shouldEqual ODataSearchResult.EmptyResult + [<Test>] let ``can detect explicit dependencies for Rx-PlaformServices``() = - parse "NuGetOData/Rx-PlatformServices.xml" - |> shouldEqual - { PackageName = "Rx-PlatformServices" + parseList "NuGetOData/Rx-PlatformServices.xml" + |> shouldEqual + ({ PackageName = "Rx-PlatformServices" DownloadUrl = "https://www.nuget.org/api/v2/package/Rx-PlatformServices/2.3.0" SerializedDependencies = [PackageName "Rx-Interfaces",DependenciesFileParser.parseVersionRequirement(">= 2.2"), "true" @@ -44,12 +53,13 @@ let ``can detect explicit dependencies for Rx-PlaformServices``() = Version = "2.3.0" CacheVersion = NuGet.NuGetPackageCache.CurrentCacheVersion SourceUrl = fakeUrl } + |> ODataSearchResult.Match) [<Test>] let ``can detect explicit dependencies for EasyNetQ``() = - parse "NuGetOData/EasyNetQ.xml" + parseList "NuGetOData/EasyNetQ.xml" |> shouldEqual - { PackageName = "EasyNetQ" + ({ PackageName = "EasyNetQ" DownloadUrl = "https://www.nuget.org/api/v2/package/EasyNetQ/0.40.3.352" SerializedDependencies = [PackageName "RabbitMQ.Client",DependenciesFileParser.parseVersionRequirement(">= 3.4.3"),"true"] @@ -58,10 +68,11 @@ let ``can detect explicit dependencies for EasyNetQ``() = CacheVersion = NuGet.NuGetPackageCache.CurrentCacheVersion Version = "0.40.3.352" SourceUrl = fakeUrl } + |> ODataSearchResult.Match) [<Test>] let ``can detect explicit dependencies for Fleece``() = - parse "NuGetOData/Fleece.xml" + parseEntry "NuGetOData/Fleece.xml" |> shouldEqual { PackageName = "Fleece" DownloadUrl = "http://www.nuget.org/api/v2/package/Fleece/0.4.0" @@ -78,7 +89,7 @@ let ``can detect explicit dependencies for Fleece``() = [<Test>] let ``can detect explicit dependencies for ReadOnlyCollectionExtensions``() = - let item = parse "NuGetOData/ReadOnlyCollectionExtensions.xml" + let item = parseEntry "NuGetOData/ReadOnlyCollectionExtensions.xml" item |> shouldEqual { PackageName = "ReadOnlyCollectionExtensions" @@ -94,9 +105,9 @@ let ``can detect explicit dependencies for ReadOnlyCollectionExtensions``() = [<Test>] let ``can detect explicit dependencies for Math.Numerics``() = - parse "NuGetOData/Math.Numerics.xml" + parseList "NuGetOData/Math.Numerics.xml" |> shouldEqual - { PackageName = "MathNet.Numerics" + ({ PackageName = "MathNet.Numerics" DownloadUrl = "http://www.nuget.org/api/v2/package/MathNet.Numerics/3.3.0" Unlisted = false Version = "3.3.0" @@ -105,10 +116,11 @@ let ``can detect explicit dependencies for Math.Numerics``() = SerializedDependencies = [PackageName "TaskParallelLibrary",DependenciesFileParser.parseVersionRequirement(">= 1.0.2856"), "&& (>= net35) (< net40)" ] SourceUrl = fakeUrl } + |> ODataSearchResult.Match) [<Test>] let ``can detect explicit dependencies for Math.Numerics.FSharp``() = - (parse "NuGetOData/Math.Numerics.FSharp.xml")|> NuGet.NuGetPackageCache.getDependencies |> Seq.head + (parseList "NuGetOData/Math.Numerics.FSharp.xml") |> ODataSearchResult.get |> NuGet.NuGetPackageCache.getDependencies |> Seq.head |> shouldEqual (PackageName "MathNet.Numerics", DependenciesFileParser.parseVersionRequirement("3.3.0"),makeOrList []) @@ -120,7 +132,7 @@ let ``can calculate v3 path``() = [<Test>] let ``can detect explicit dependencies for Microsoft.AspNet.WebApi.Client``() = - let odata = parse "NuGetOData/Microsoft.AspNet.WebApi.Client.xml" + let odata = parseList "NuGetOData/Microsoft.AspNet.WebApi.Client.xml" |> ODataSearchResult.get odata.PackageName |> shouldEqual "Microsoft.AspNet.WebApi.Client" odata.DownloadUrl |> shouldEqual"https://www.nuget.org/api/v2/package/Microsoft.AspNet.WebApi.Client/5.2.3" let dependencies = odata|> NuGet.NuGetPackageCache.getDependencies |> Array.ofList @@ -134,7 +146,7 @@ let ``can detect explicit dependencies for Microsoft.AspNet.WebApi.Client``() = [<Test>] let ``can detect explicit dependencies for WindowsAzure.Storage``() = - let odata = parse "NuGetOData/WindowsAzure.Storage.xml" + let odata = parseList "NuGetOData/WindowsAzure.Storage.xml" |> ODataSearchResult.get odata.PackageName |> shouldEqual "WindowsAzure.Storage" odata.DownloadUrl |> shouldEqual"https://www.nuget.org/api/v2/package/WindowsAzure.Storage/4.4.1-preview" let dependencies = odata|> NuGet.NuGetPackageCache.getDependencies |> Array.ofList @@ -156,7 +168,7 @@ let ``can detect explicit dependencies for WindowsAzure.Storage``() = [<Test>] let ``can ignore unknown frameworks``() = - let parsed = parse "NuGetOData/BenchmarkDotNet-UnknownFramework.xml" + let parsed = parseList "NuGetOData/BenchmarkDotNet-UnknownFramework.xml" |> ODataSearchResult.get parsed |> shouldEqual { PackageName = "BenchmarkDotNet" diff --git a/tests/Paket.Tests/Paket.Tests.fsproj b/tests/Paket.Tests/Paket.Tests.fsproj index 2b66d44952..8e9bc3fe88 100644 --- a/tests/Paket.Tests/Paket.Tests.fsproj +++ b/tests/Paket.Tests/Paket.Tests.fsproj @@ -135,6 +135,9 @@ <Content Include="NuGetOData\Fleece.xml"> <CopyToOutputDirectory>Always</CopyToOutputDirectory> </Content> + <Content Include="NuGetOData\EmptyFeedList.xml"> + <CopyToOutputDirectory>Always</CopyToOutputDirectory> + </Content> <Content Include="NuGetOData\ReadOnlyCollectionExtensions.xml"> <CopyToOutputDirectory>Always</CopyToOutputDirectory> </Content> From 7c881aeec958898418c39176cb140d6f94291a61 Mon Sep 17 00:00:00 2001 From: Matthias Dittrich <matthi.d@gmail.com> Date: Mon, 19 Jun 2017 23:51:22 +0200 Subject: [PATCH 2/2] fix build of integrationtest project --- integrationtests/Paket.IntegrationTests/PackSpecs.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/integrationtests/Paket.IntegrationTests/PackSpecs.fs b/integrationtests/Paket.IntegrationTests/PackSpecs.fs index c1814ee60c..7da4e3a460 100644 --- a/integrationtests/Paket.IntegrationTests/PackSpecs.fs +++ b/integrationtests/Paket.IntegrationTests/PackSpecs.fs @@ -166,6 +166,7 @@ let ``#1429 pack deps without minimum-from-lock-file uses specifc dependencies f let details = NuGetLocal.getDetailsFromLocalNuGetPackage false None outPath "" (PackageName "PaketBug") (SemVer.Parse "1.0.0.0") |> Async.RunSynchronously + |> ODataSearchResult.get details |> getDependencies |> List.map (fun (x,_,_) -> x) |> shouldContain (PackageName "MySql.Data") let packageName, versionRequirement, restrictions = details |> getDependencies |> List.filter (fun (x,_,_) -> x = PackageName "MySql.Data") |> List.head