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

Fix to stop all updates when only one app failed to update. #3628

Closed
wants to merge 2 commits into from

Conversation

lyuha
Copy link

@lyuha lyuha commented Sep 5, 2019

No description provided.

When update all apps, only one app failed to update, it terminates
and stops other apps' update. Fix this problem to add try-catch
statement
# $outdated is a list of ($app, $global) tuples
$outdated | ForEach-Object { update @_ $quiet $independent $suggested $use_cache $check_hash }
$outdated | ForEach-Object { try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$outdated | ForEach-Object { try {
$outdated | ForEach-Object {
try {

$outdated | ForEach-Object { try {
update @_ $quiet $independent $suggested $use_cache $check_hash
} catch {
Write-Error $_
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Write-Error $_
error $_

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you want to show in this error? Name of manifest or error message?

Copy link
Author

Choose a reason for hiding this comment

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

I want show error including error message for report issue.

@Ash258
Copy link
Contributor

Ash258 commented Sep 5, 2019

I do not think this will work as there are abort calls.

@Ash258
Copy link
Contributor

Ash258 commented Sep 5, 2019

# File B.ps1
function Abort-Mock {
    write-host 'before calling exit'
    exit 1
}

# File A.ps1
. $psscriptroot\B.ps1

try {
	Abort-Mock
} catch {
    'ahoj'
}

Write-host 'after exit call'

Result is always only message "Before calling exit"

@lyuha
Copy link
Author

lyuha commented Sep 5, 2019

Thanks to review.

I can't find some way to trap or catch abort calls. But, why handles abort calls at same level process? exit 1 means to terminate process and report error to parent process. It does not use to handle in same level.

@Ash258
Copy link
Contributor

Ash258 commented Sep 5, 2019

I am just saying that this will not fix problem when there is some hash check fail or installation error. Because on hash check fail there is abort call which have exit 1, which will stop whole scoop process all the time with or without try catch block. So this "fix" will not fix anything.

@lyuha
Copy link
Author

lyuha commented Sep 5, 2019

Good description.

I checked codes to find error I met. Found it

If found new version app, scoop try download it using dl_with_cache function. dl_with_cache function calls do_dl. Inside do_dl function, it has try-catch and throw $e. This error is not handled while update.

This changes fix above problem. However I feels this pr is not right way.

Why do_dl catch error and throw agian?

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

Successfully merging this pull request may close these issues.

2 participants