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

Update fix for Issue #555 #559

Merged
merged 8 commits into from
Jun 30, 2019
Merged

Update fix for Issue #555 #559

merged 8 commits into from
Jun 30, 2019

Conversation

abraunegg
Copy link
Owner

* Update fix for #555 as try | catch block for session creation appears to miss error response codes
@abraunegg abraunegg requested a review from norbusan June 27, 2019 20:36
norbusan
norbusan previously approved these changes Jun 28, 2019
Copy link
Collaborator

@norbusan norbusan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine - when one sets up not to show whitespace changes in github, then it becomes clear so easily ;-) Thanks

* Udate handling when response is not a valid JSON object
@abraunegg abraunegg requested a review from norbusan June 28, 2019 03:14
@abraunegg
Copy link
Owner Author

Looks fine - when one sets up not to show whitespace changes in github, then it becomes clear so easily ;-) Thanks

yep ... wish there was a way to make that the default!

norbusan
norbusan previously approved these changes Jun 28, 2019
@abraunegg
Copy link
Owner Author

@norbusan
will wait for user confirmation of fix before merging incase additional changes needed

* remove try block and trap for explicit entry earlier
* Update how JSONValue object is determined to be valid
* Add error logging when response is not a valid JSON object
* Clean-up messaging and error logging
@abraunegg
Copy link
Owner Author

@norbusan
Finally able to reproduce the error and problem & validate this this PR resolves the issue:

Remaining free space: 1092956070708
Uploading new file ./random_files/ysDocMar1J4Qquv4GHaGGzJ9Dml1JfLO/file8.data ...
Uploading 100% |oooooooooooooooooooooooooooooooooooooooo|   ETA   --:--:--:                                                                                                             
 done.
Remaining free space: 1092954289716
Uploading new file ./random_files/ysDocMar1J4Qquv4GHaGGzJ9Dml1JfLO/file9.data ...
OneDrive returned a 'HTTP 504 Gateway Timeout Error' - gracefully handling error
Create file upload session failed ... skipping file upload
 error
Remaining free space: 1092950351156
Uploading new file ./random_files/ysDocMar1J4Qquv4GHaGGzJ9Dml1JfLO/file10.data ...
Uploading 100% |oooooooooooooooooooooooooooooooooooooooo| DONE IN 00:00:06                                                                                                              
 done.

On the next run:

The file has not changed
Uploading new items of .
Uploading new file ./random_files/82iEA9LsZM229lMS6B3TweynX9jQFygL/file22.data ...
Uploading 100% |oooooooooooooooooooooooooooooooooooooooo|   ETA   --:--:--:                                                                                                             
 done.
Remaining free space: 1092894471553
Uploading new file ./random_files/ysDocMar1J4Qquv4GHaGGzJ9Dml1JfLO/file9.data ...
Uploading 100% |oooooooooooooooooooooooooooooooooooooooo|   ETA   --:--:--:                                                                                                             
 done.
Remaining free space: 1092890532993
Applying changes of Path ID: 01MBBT6UF6Y2GOVW7725BZO354PWSELRRZ
Processing 6 changes

This PR fixes:

  • Key not found: uploadUrl
  • JSONValue is not an object when OneDrive generates an error that is not correctly trapped by a try block

@abraunegg abraunegg requested a review from norbusan June 29, 2019 06:10
* Simplify check to one statement
Copy link
Collaborator

@norbusan norbusan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So return values sometimes are not json objects, that is a pain indeed. Thanks for fixing (and great that you can reproduce it!)

@abraunegg abraunegg merged commit 4a4611c into master Jun 30, 2019
@abraunegg abraunegg deleted the Issue-#555-v2 branch June 30, 2019 05:18
@lock
Copy link

lock bot commented Jul 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants