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.

Sign up for GitHub

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

Jump to bottom

Fix unhandled error exceptions that cause application to exit / crash when uploading files (Issue #624) #625

Merged
merged 9 commits into from
Aug 24, 2019
Merged

Fix unhandled error exceptions that cause application to exit / crash when uploading files (Issue #624) #625

merged 9 commits into from
Aug 24, 2019

Conversation

abraunegg
Copy link
Owner

  • remove 'else throw e' which causes the application to exit / crash and replace with what the actual error message is from OneDrive

@abraunegg
Fix unhandled error exceptions that cause application to exit / crash
cdffc9b 
* remove 'else throw e' which causes the application to exit / crash and replace with what the actual error message is from OneDrive
@abraunegg abraunegg changed the title WIP: Fix unhandled error exceptions that cause application to exit / crash WIP: Fix unhandled error exceptions that cause application to exit / crash (Issue #624) Aug 21, 2019
@abraunegg
Update sync.d
985eb18 
* Make logging of unhandled errors consistent
@abraunegg
Update sync.d
f9d31d8 
* Update error message handling
* Add file system exception handling
* Add 401 Unauthorised response code check
* Add try & catch for modified file uploads
@abraunegg abraunegg added this to the v2.3.9 milestone Aug 22, 2019
@abraunegg
Merge branch 'master' into Issue-#624
7b08423
@abraunegg
Update sync.d
b521ebb 
* Add extra error handling when uploading files for errors received from OneDrive or file system errors
@abraunegg
Update sync.d
f58dbb5 
* Flag that the upload failed if there is an exception error raised
@abraunegg
Update sync.d
16da075 
* Update logging output for when an error occurs to indicate that the file was skipped
* Log the 401 response as log.vlog rather than log.error as it is now being handled correctly
@abraunegg
Copy link
Owner Author

@norbusan
Without this patch, when uploading to OneDrive with a personal account sometimes OneDrive returns a 401 error which does not get handled. Finally able to reproduce this when testing this PR:

Uploading new file ./random_files_2/random_files_dir_set_1/7BeaLQ0WlcxJFtZ6hWER1InpEs4nME7A/file904.data ... done.
Remaining free space: 4690224151
Uploading new file ./random_files_2/random_files_dir_set_1/7BeaLQ0WlcxJFtZ6hWER1InpEs4nME7A/file955.data ...ERROR: OneDrive returned a 'HTTP 401 - Unauthorized' - gracefully handling error
Uploading new file ./random_files_2/random_files_dir_set_1/7BeaLQ0WlcxJFtZ6hWER1InpEs4nME7A/file812.data ... done.
Remaining free space: 4690210807
Uploading new file ./random_files_2/random_files_dir_set_1/7BeaLQ0WlcxJFtZ6hWER1InpEs4nME7A/file203.data ... done.
Remaining free space: 4690180224

@abraunegg abraunegg changed the title WIP: Fix unhandled error exceptions that cause application to exit / crash (Issue #624) Fix unhandled error exceptions that cause application to exit / crash (Issue #624) Aug 23, 2019
@abraunegg abraunegg changed the title Fix unhandled error exceptions that cause application to exit / crash (Issue #624) WIP: Fix unhandled error exceptions that cause application to exit / crash when uploading files (Issue #624) Aug 23, 2019
@abraunegg abraunegg changed the title WIP: Fix unhandled error exceptions that cause application to exit / crash when uploading files (Issue #624) Fix unhandled error exceptions that cause application to exit / crash when uploading files (Issue #624) Aug 23, 2019
This was referenced Aug 23, 2019
Merged
Merged
Merged
@abraunegg
update error message output and catch 50x errors
c1d2f23 
* Fix up error message output so when other errors occur
* Throw an exception for 50x errors so that it gets caught correctly
@abraunegg
Copy link
Owner Author

Fix this sort of issue as well:

Remaining free space: 4648337423
Uploading new file ./random_files_2/random_files_dir_set_1/VZfhaIwH60BG0ik4kRw4ZX5rc4DIXuub/file280.data ...OneDrive returned a 'HTTP 503 Service Unavailable Error' - gracefully handling error
 done.
ERROR: An error was returned from OneDrive and the resulting response is not a valid JSON object
ERROR: Increase logging verbosity to assist determining why.
Remaining free space: 4648333814
Uploading new file ./random_files_2/random_files_dir_set_1/VZfhaIwH60BG0ik4kRw4ZX5rc4DIXuub/file986.data ... done.

The 503 should be handled, upload should be listed as skipped and logging output should not be jammed up together

@abraunegg
Copy link
Owner Author

Logging example with latest commit:

Uploading new file ./random_files_2/random_files_dir_set_3/6VEzVEgmf3nJAiNllq4tZpLd2v50HQHe/file399.data ... done.
Uploading new file ./random_files_2/random_files_dir_set_3/6VEzVEgmf3nJAiNllq4tZpLd2v50HQHe/file372.data ... skipped.

ERROR: OneDrive returned an error with the following message:
  Error Message: HTTP request returned status code 500 (Internal Server Error)
  Error Reason:  An error occurred in the data store.
Uploading new file ./random_files_2/random_files_dir_set_3/6VEzVEgmf3nJAiNllq4tZpLd2v50HQHe/file667.data ... done.
Uploading new file ./random_files_2/random_files_dir_set_3/6VEzVEgmf3nJAiNllq4tZpLd2v50HQHe/file947.data ... done.
Uploading new file ./random_files_2/random_files_dir_set_3/6VEzVEgmf3nJAiNllq4tZpLd2v50HQHe/file872.data ... done.

@abraunegg
Update sync.d
a3306dc 
* Clean up logging when exception is generated
norbusan
norbusan approved these changes Aug 24, 2019
View reviewed changes
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.

Code looks fine, thanks for refactoring the logging. Compile and run tested. But I never could trigger the error myself.

@norbusan
Copy link
Collaborator

@abraunegg I tried to restart the analysis, didn't work either. Something is strange wioth codacy atm.

@abraunegg abraunegg merged commit 488f756 into master Aug 24, 2019
@abraunegg abraunegg deleted the Issue-#624 branch August 24, 2019 05:54
@abraunegg abraunegg mentioned this pull request Aug 24, 2019
Closed
abraunegg added a commit that referenced this pull request Aug 24, 2019
@abraunegg
Fix merge issues after #625
d4eeb7e 
* Fix merge issues after #625
@lock
Copy link

lock bot commented Sep 23, 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 Sep 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@norbusan norbusan norbusan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.3.9
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@abraunegg @norbusan