-
Notifications
You must be signed in to change notification settings - Fork 253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot download multiple OneDrive files concurrently since v.1.13.0-beta #413
Comments
Thank you for reporting this issue and providing repro steps.. We will investigate this. |
@tipa, We have released a fix for this issue as part of release 1.14.1. Give it a try and let us know if it addresses your scenario. |
I am struggling including the project into my solution. Would it be possible to release it on Nuget.org as a preview package or make the NuGet file available to me as a direct download? |
My reading of that trace is that request 53-59 + 62-63 are the initial requests that redirect with a 302 and then 66,67,70,71,etc are the corresponding GETs to the redirect location with response payload and .200. Is there a reason you believe this is not the case? |
No, this is absolutely correct. As I wrote in the first post, the first ~10-15 files are downloaded correctly. But you then see the requests in line 86 ff: They redirect to a different URL, but the library never sends a GET requests to them |
@tipa My apologies, I missed that detail in your original post. I will continue to try to repro your issue. Could you try performing the multiple downloads by simply accumulating the tasks into an array and doing WaitAll and see if you still experience the problem. I'm assuming the semaphone/Task.Run/try/catch is a way of aborting all the tasks if one fails. If so you might be interested in this approach https://gist.github.com/svick/9992598 which uses a cancellation token to cancel all other tasks if one fails. |
@tipa When just loading tasks into an array and doing WaitAll I am able to download 25 files without any issue or slowdown. |
@tipa Do you have AppInsights enabled on your app? In the stacktrace I see a reference to DiagnosticsHandler and I suspect that might be coming from AppInsights. |
Tried that (I simply commented out the
The main reason for using the sema is to only download 10 files at a time. Sometimes I need to download multiple hundreds or even thousands of files which would lead to timeout errors very quickly if I requested them all at once. The try-catch is for the case a task fails and to not make any additional requests. I don't think the semaphore is the cause of the issue here because it worked fine with v1.1.12 and with other cloud storage libraries I implemented (Google Drive, Dropbox, ...)
I am using AppCenter in my app |
Can you share with us a capture of using 1.14.1 getting redirected but not sending the subsequent GET requests? Can you show us the same capture using 1.12 with the same settings? |
Manage the Microsoft.Graph.Core NuGet and select Version 1.14.1 from the version drop down. |
Yes, we can update Microsoft.Graph to use 1.14.1 of Core as the minimum dependency but then it forces an upgrade. We need to change packagereference to use a wildcard. |
Seems like I am not the only one having that issue: #421 (comment) |
I am running into the same issue. Upgraded to version 1.14. and core 1.14.1 and onedrive download slows down and breaks with timout service request exception. What is the proposed solution? |
@whentotrade Are you also using Task.Run to execute parrallel calls? Would you be able to provide a snippet of the code you are using for downloading files? |
@darrelmiller: Yes. Snipped below. The same code worked without any problem with 1.12. However with 1.14. the try/catch in the GetFileBytes function is raising the exception with "graph service exception" timeout somewhere around 100 files. The first files run smooth, then slows down and breaks. Added note: Just running two tasks in parallel, not more.
|
@whentotrade Thanks, this is really helpful. |
Here is the error log catched:
|
@whentotrade Are you experiencing this in a UWP or Xamarin? |
I experienced the issue on all UWP + Xamarin.Android + Xamarin.iOS |
UWP (WPF .NET with Desktop Bridge as UWP) |
@tipa @whentotrade Thanks for the feedback. We've found what's causing the issue and we are currently working on a fix. |
@tipa, @whentotrade We've released Microsoft.Graph.Core 1.15.0-preview.2 that should fix this issue. Give it a try and let us know if it addresses your scenario. |
Thanks! Can you please update the Microsoft.Graph nuget package so the issue described above does not happen? |
using Graph 1.14 and Core 1.15.0-preview.2 for quick test: Problem solved, looks like issue is fixed. Thank you and keep up your good work and service! |
@whentotrade How did you manage to force-use Core 1.15.0-preview.2 instead of 1.14? |
@tipa: Yes. I first just upgraded the Core manually to 1.15.0-preview.2 and afterwards I upgraded the Graph package to 1.14. This way the upgrade for the Graph didnt requested the Core dependency package and accepted the already available 1.15 alpha.2. |
Thanks! But that sounds like it would grab 1.14 again after a project clean&rebuild... Tried it anyways and the problem was not solved for me on Android.Xamarin. Not sure if it used 1.14 anyways, but the .dll in the \obj\Debug\90\android\assets folder indicated v.1.15 |
The issue is fixed for me using .NET WPF Desktop App with UWP. Final question: |
@whentotrade Yay! Great to hear your issue was resolved. Using Microsoft.Graph 1.14 and Microsoft.Graph.Core 1.15.0-preview.2 is valid combination, although we don't recommend using any "preview" build in production. We expect to release a final version of 1.15 this week. We are starting to allow the versions of the "service library" Microsoft.Graph that contains all the generated code, to diverge from the "core library". Currently the numbers are close so it looks like they should align. However, moving forward they will be released independently and versioned independently. @tipa You should be able to update to the preview core package by doing
Let us know if you contain to have problems. |
Bug still present with v1.15.0 on Xamarin.Android & Xamarin.iOS |
We are investigating this today. |
Any updates? |
@tipa We internally use // Use AndroidClientHandler as opposed to HttpClientHandler.
var innerHandler = new AndroidClientHandler { AllowAutoRedirect = false };
var pipeline = GraphClientFactory.CreatePipeline(GraphClientFactory.CreateDefaultHandlers(authProvider), innerHandler);
var graphClient = new GraphServiceClient(authProvider, new HttpProvider(pipeline, true, new Serializer())); Alternatively, you can clone this sample that I've setup and run it against your test data and see if it works for your scenario. |
That works for Android, but I am still having problems with iOS. My code looks like this now:
I am getting this exception:
I am getting a similar exception on Android, when I initialize the AndroidClientHandler like this: |
@tipa Thanks for the detailed log! For now, in your iOS preprocessor directive, you can remove our default #elif __IOS__
var innerHandler = new NSUrlSessionHandler { AllowAutoRedirect = false };
var handlers = GraphClientFactory.CreateDefaultHandlers(authProvider);
// Remove CompressionHandler from pipeline when using NSUrlSessionHandler.
handlers.Remove(handlers.FirstOrDefault((h) => h.GetType().Equals(typeof(CompressionHandler)))); // We need to provide a method to remove handlers by type.
var pipeline = GraphClientFactory.CreatePipeline(handlers , innerHandler);
httpProvider = new HttpProvider(pipeline, true, new Serializer()); With this change, i'm able to download all files in parallel without an exception. Let us if it works for you. |
Yaaay, that made it work! Thanks a lot - finally I can use v1.15.0 in all 3 projects. |
@peombwa @darrelmiller I started buidling a Xamarin.macOS app recently and now experience this issue again. Would it be possible to also use NSUrlSession by default for macOS? Right only the native handlers are used for Android and iOS only: https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/blob/ada45005494a1e274a3a7db5a167ab3e0bc3940a/src/Microsoft.Graph.Core/Requests/GraphClientFactory.cs#L217 |
@tipa That seems like a reasonable request. I'm guessing it would require adding in an extra platform check. If you had the time to make a PR to add this, I can get it merged. If not, I'll try and get a dev to make the fix before our next release. Unfortunately, most of our devs are tied up with other priorities at the moment. |
I created this pull request here: microsoftgraph/msgraph-sdk-dotnet-core#99 |
@tipa Thank you. We will investigate the issue. |
Expected behavior
Until and including v1.12.0 I was able to download many files from OneDrive concurrently (see code snippet below) without any issues (and fast!).
Actual behavior
Since v1.13.0-beta (and incl. the now available v1.14.0) the download of multiple OneDrive files is slow from the beginning and is getting even slower over time. The app can only download up to ~15 files before the process eventually stops completely and the following exception is raised (no custom timeout set):
Steps to reproduce the behavior
Microsoft.Graph v1.14.0
UWP app (but also happens in Xamarin)
The text was updated successfully, but these errors were encountered: