Improve error reporting #2423

matthid · 2017-06-15T11:52:35Z

This improves output and internal error propagation in the NuGet-RequestVersions code.
The goal is to output in the error case which urls where requested and what reponse we got from them...

For now this is WIP, because I think there might be some cases now where we print way too much info (in the non-verbose case).

…ting

…out (to get an idea what the process was doing).

…via AggregateException.

…e message.

matthid

Let's see what the CI says.

matthid · 2017-06-15T14:24:52Z

src/Paket.Core/Common/Async.fs

-    static member Choice(tasks : Async<'T option> seq) = async {
+    static member map f a =
+        async { return f a }
+    static member tryFind (f : 'T -> bool) (tasks : Async<'T> seq) = async {


This is an interesting API.
It is almost like Async.Choice, but will return all the started tasks and an index which one matched the given function.

matthid · 2017-06-15T14:29:40Z

src/Paket.Core/Common/Utils.fs

@@ -560,7 +555,7 @@ let downloadFromUrl (auth:Auth option, url : string) (filePath: string) =
            do! task
        with
        | exn ->
-            failwithf "Could not download from %s%s Message: %s%s" url Environment.NewLine exn.Message (innerText exn)
+            raise <| Exception(sprintf "Could not download from '%s'" url, exn)


We almost never should do failwithf in a general with exn -> clause, not even when we embed the message (like here)...
I really think F# should make this more easy (with some syntax), even the F# compiler is doing it wrong everywhere

/cc @dsyme any ideas?
maybe allowing to use comma in combination with failwithf "some message", innerException

matthid · 2017-06-15T14:30:35Z

src/Paket.Core/Common/Utils.fs

@@ -612,11 +606,12 @@ let safeGetFromUrl (auth:Auth option, url : string, contentType : string) =
 #endif
            use _ = Profile.startCategory Profile.Category.NuGetRequest
            let! raw = client.DownloadStringTaskAsync(uri) |> Async.AwaitTask
-            return Some raw
+            return FSharp.Core.Result.Ok raw


Maybe I should use full-out exceptions everywhere :/

matthid · 2017-06-15T16:13:48Z

This is how a failure looks now:

Paket failed with:
-> AggregateException: Could not find versions for package Argu on https://www.myget.org/F/paket-test.
-> AggregateException: Source 'https://www.myget.org/F/paket-test' yielded no results
	 - Request 'https://www.myget.org/F/paket-test/FindPackagesById()?semVerLevel=2.0.0&id='Argu'' finished with error
	-> Could not retrieve data from 'https://www.myget.org/F/paket-test/FindPackagesById()?semVerLevel=2.0.0&id='Argu''
	-> WebException: The remote server returned an error: (401) Unauthorized.
	 - Request 'https://www.myget.org/F/paket-test/Packages?semVerLevel=2.0.0&$filter=tolower(Id) eq 'argu'' finished with error
	-> Could not retrieve data from 'https://www.myget.org/F/paket-test/Packages?semVerLevel=2.0.0&$filter=tolower(Id) eq 'argu''
	-> WebException: The remote server returned an error: (401) Unauthorized.

These changes also show that the random failure we get sometimes is apparently another mono bug:

Paket failed with:
-> AggregateException: Could not find versions for package WebSharper on https://www.nuget.org/api/v2.
-> AggregateException: Source 'https://www.nuget.org/api/v2' yielded no results
	 - Request 'https://www.nuget.org/api/v2/FindPackagesById()?semVerLevel=2.0.0&id='WebSharper'' finished with error
	-> Could not retrieve data from 'https://www.nuget.org/api/v2/FindPackagesById()?semVerLevel=2.0.0&id='WebSharper''
	-> WebException: Error: SecureChannelFailure (Ssl error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER
	     at /tmp/buildd/mono-5.0.1.1/external/boringssl/ssl/tls_record.c:217)
	-> MonoBtlsException: Ssl error:100000f7:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER
	     at /tmp/buildd/mono-5.0.1.1/external/boringssl/ssl/tls_record.c:217
	 - Request 'https://www.nuget.org/api/v2/Packages?semVerLevel=2.0.0&$filter=tolower(Id) eq 'websharper'' finished with error
	-> Could not retrieve data from 'https://www.nuget.org/api/v2/Packages?semVerLevel=2.0.0&$filter=tolower(Id) eq 'websharper''
	-> WebException: Error: TrustFailure (Ssl error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
	     at /tmp/buildd/mono-5.0.1.1/external/boringssl/ssl/handshake_client.c:1132)
	-> MonoBtlsException: Ssl error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
	     at /tmp/buildd/mono-5.0.1.1/external/boringssl/ssl/handshake_client.c:1132

matthid · 2017-06-15T16:23:05Z

https://github.com/mono/boringssl/blob/mono/ssl/s3_both.c#L363

https://github.com/mono/boringssl/blob/mono/ssl/s3_both.c#L291

The record length is a 16-byte value and is formatted in network order.

In theory, this means that a single record can be up to 65,535 (2^16 -1) bytes in length. The TLSv1 RFC2246 states that the maximum length is 16,383 (2^14 -1) bytes. Microsoft products (Microsoft Internet Explorer and Internet Information Services) are known to exceed these limits.

http://www.cisco.com/c/en/us/support/docs/security-vpn/secure-socket-layer-ssl/116181-technote-product-00.html

Nice

matthid · 2017-06-15T16:53:53Z

Note that

1) Error : Paket.IntegrationTests.ConvertFromNuGetSpecs.#1217 should replace packages.config files in project
System.Exception : Paket failed with:
-> Exception: There was a version conflict during package resolution.
     Conflict detected:
      - Dependencies file requested package Newtonsoft.Json: 7.0.1
      - Available versions:
        - (10.0.2, [https://www.nuget.org/api/v2])
       ... snip
        - (8.0.1-beta1, [https://www.nuget.org/api/v2])
        - (7.0.1, [https://dotnet.myget.org/F/cli-deps; https://www.nuget.org/api/v2])
        - (7.0.1-beta3, [https://www.nuget.org/api/v2])
        - (7.0.1-beta2, [https://www.nuget.org/api/v2])
        - (7.0.1-beta1, [https://www.nuget.org/api/v2])
        - (6.0.8, [https://www.nuget.org/api/v2])
       ... snip
        - (3.5.8, [https://www.nuget.org/api/v2])
   
     Please try to relax some conditions or resolve the conflict manually (see http://fsprojects.github.ioPaket/nuget-dependencies.html#Use-exactly-this-version-constraint).

Is explained by the mono bug as well:
I think the request to GetPackageDetails was failing.
Paket needs to improve this error message to include this (important) detail...

… fails.

…ed by the structure)

matthid · 2017-06-15T19:33:35Z

There are so many more places where with _ -> is used throughout the paket code base, but I'm happy with this subset for now as especially in the NuGet request logic it was often hard to figure out what was going on.

…es in the final exception print (might be useful for single tests or to tell people to get better bug reports).

forki · 2017-06-16T07:24:02Z

is this ready?

forki · 2017-06-16T08:15:51Z

thx!

matthid added 14 commits June 13, 2017 22:06

partial work...

117a1ea

Merge remote-tracking branch 'origin/master' into improve_error_repor…

8df0ed5

…ting

finish initial proper error reporting

6e74c7c

fix some random change

d315ec9

fix another random change

ea32f48

fix Async.tryFind in special case of single item.

a20a6c5

change integration test suite to print output when the process timed …

fa2f230

…out (to get an idea what the process was doing).

print more exception informations & use this to forward NuGet errors …

966b38e

…via AggregateException.

reduce nesting via ->

b4f9550

add missing newline

bf2abbc

fix Installtion Errors message

6d10d98

properly forward exception details.

88f6b3a

fix newline location

ecc8be4

' - ' is already given by the exception structure, remove it from th…

5e16733

…e message.

matthid commented Jun 15, 2017

View reviewed changes

matthid added 3 commits June 15, 2017 17:03

generalize error handling and improve aggregateexception output

c80c66a

fix endless loop

9224d4c

skip 'Exception: ' as it doesn't yield any information.

07f320b

matthid changed the title ~~[WIP] Improve error reporting~~ Improve error reporting Jun 15, 2017

matthid added 2 commits June 15, 2017 18:44

handle some other cases where error information is hidden

0025b85

minor output improvement.

79736fd

matthid added 5 commits June 15, 2017 19:28

track errors into the conflict-state and print them out if resolution…

ee7d1ff

… fails.

use a different strategy and warn when resolution was OK.

53c2063

fix a bug in the runtime resolution.

73bce52

minor output improvement.

e8f3067

remove 'AggregateException: ' as well (this information can be inferr…

3cf9654

…ed by the structure)

add 'PAKET_DETAILED_ERRORS' feature, this will always print stacktrac…

c00d52d

…es in the final exception print (might be useful for single tests or to tell people to get better bug reports).

This was referenced Jun 15, 2017

Copy missing changes from legacy NuGet helpers fsprojects/FAKE#1596

Merged

#2419. Try parse SemVerInfo from .nupkg file name for localFolder nuget source #2422

Merged

forki merged commit 4904b5a into master Jun 16, 2017

forki deleted the improve_error_reporting branch June 16, 2017 08:15

matthid mentioned this pull request Jun 17, 2017

improve another error when restoring packages. #2434

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error reporting #2423

Improve error reporting #2423

matthid commented Jun 15, 2017

matthid left a comment

matthid Jun 15, 2017

matthid Jun 15, 2017

matthid Jun 15, 2017

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

forki commented Jun 16, 2017

forki commented Jun 16, 2017

Improve error reporting #2423

Improve error reporting #2423

Conversation

matthid commented Jun 15, 2017

matthid left a comment

Choose a reason for hiding this comment

matthid Jun 15, 2017

Choose a reason for hiding this comment

matthid Jun 15, 2017

Choose a reason for hiding this comment

matthid Jun 15, 2017

Choose a reason for hiding this comment

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

matthid commented Jun 15, 2017

forki commented Jun 16, 2017

forki commented Jun 16, 2017