Skip to content
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

Get NotSupportedException for a non-seekable stream when ResumeAsync is called after previous call on ResumeAsync timeout #602

Closed
sparkinglu opened this issue Sep 22, 2015 · 4 comments
Assignees

Comments

@sparkinglu
Copy link

Not sure if this is a real issue.

When the resumable sync is used with a non-seekable stream, we got the NotSupportedException sometimes. We went through the codes and found there may be something wrong in the ResumableUpload.cs.

  1. At the 374th line (StreamLength = ContentStream.CanSeek ? ContentStream.Length : UnknownSize;). the StreamLength is the UnknownSize if the stream is non-seekable.
  2. So at the 517th-527th lines, it seems correct to use StreamLength to check if the stream is seekable. When it is seekable, it calls PrepareNextChunkKnownSize which calls set positions at 652th.
    if (StreamLength != UnknownSize)
    {
    PrepareNextChunkKnownSize(request, stream, cancellationToken);
    }
    else
    {
    PrepareNextChunkUnknownSize(request, stream, cancellationToken);
    }
  3. However at the 627th (StreamLength = LastMediaLength + BytesServerReceived;), the StreamLength is assigned always even though it is non-seekable. So it could be wrong to use the StreamLength to check if it is seekable if the previous upload timeout and one retry is done on the same ResumableUpload instance.
@sparkinglu
Copy link
Author

Here are the pseudo-codes to reproduce this issue when the uploaded file size is less than chunk size and the 1st call on ResumeAsync timeouts.

create an object of ResumableUpload.InsertMediaUpload
retry the below operations if timeout
calls ResumeAsync
return if complete

@peleyal
Copy link
Collaborator

peleyal commented Oct 18, 2015

Sorry for the late response....

  1. So, your stream is non-seekable, right? So, StreamLength is set to unknown size.
    https://github.com/google/google-api-dotnet-client/blob/master/Src/GoogleApis/Apis/%5BMedia%5D/Upload/ResumableUpload.cs#L374
  2. Why do you care about seekable in this case, I thought we said that the stream is NOT seekable. So, I would expect a call to PrepareNextChunkUnknownSize,
    https://github.com/google/google-api-dotnet-client/blob/master/Src/GoogleApis/Apis/%5BMedia%5D/Upload/ResumableUpload.cs#L523
  3. PrepareNextChunkUnknownSize is defined in https://github.com/google/google-api-dotnet-client/blob/master/Src/GoogleApis/Apis/%5BMedia%5D/Upload/ResumableUpload.cs#L571.
    Are you suggesting that there is a bug in this function?
  4. In your last comment you said that the file size is less than chunk size (but it's unknown, right?)
    So is it the use case where we have a unknown size stream and the first request fails?

Thanks,
Eyal

@chrisdunelm
Copy link
Contributor

I'm fairly sure I've reproduced this bug in a test. I hope to have the new test-cases and a fix ready in the next few days.
It occurs, as you say, when the last chunk of a non-seekable stream fails to upload.

@chrisdunelm chrisdunelm self-assigned this Jun 13, 2016
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 13, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 15, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 15, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 20, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 20, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 20, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 20, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 30, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 30, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 30, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jun 30, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
chrisdunelm added a commit to chrisdunelm/google-api-dotnet-client that referenced this issue Jul 1, 2016
Test more edge-cases of ResumableUpload, and test using an HTTP server fixing googleapis#735
Fix bugs
* googleapis#755 Incorrect resume behaviour if first upload chunk fails.
* googleapis#602 Resume fails with non-seekable stream when last chunk fails to upload.
And adds test for resuming upload on program restart.

Don't run most ImprovedResumableUploadTests on travis, as they fail on the Mono infrastructure.
All tests pass on a Windows machine with MS .NET
@chrisdunelm
Copy link
Contributor

Finally fixed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants