From d089cfe8ca09c4ed8909f5fefa81ee95284e87dc Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Mon, 30 May 2022 21:00:50 +0100 Subject: [PATCH 1/2] Add some release asset upload retrying. --- internal/push/push.go | 73 +++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/internal/push/push.go b/internal/push/push.go index e69139c..130f753 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -308,37 +308,56 @@ func (pushService *pushService) uploadReleaseAsset(release *github.RepositoryRel } func (pushService *pushService) createOrUpdateReleaseAsset(release *github.RepositoryRelease, existingAssets []*github.ReleaseAsset, assetPathStat os.FileInfo) error { - for _, existingAsset := range existingAssets { - if existingAsset.GetName() == assetPathStat.Name() { - actualSize := int64(existingAsset.GetSize()) - expectedSize := assetPathStat.Size() - if actualSize == expectedSize { - return nil - } else { - log.Warnf("Removing existing release asset %s because it was only partially-uploaded (had size %d, but should have been %d)...", existingAsset.GetName(), actualSize, expectedSize) - response, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID()) - if err != nil { - return githubapiutil.EnrichResponseError(response, err, "Error deleting existing release asset.") + attempt := 0 + for { + attempt++ + for _, existingAsset := range existingAssets { + if existingAsset.GetName() == assetPathStat.Name() { + actualSize := int64(existingAsset.GetSize()) + expectedSize := assetPathStat.Size() + if actualSize == expectedSize { + return nil + } else { + log.Warnf("Removing existing release asset %s because it was only partially-uploaded (had size %d, but should have been %d)...", existingAsset.GetName(), actualSize, expectedSize) + response, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID()) + if err != nil { + return githubapiutil.EnrichResponseError(response, err, "Error deleting existing release asset.") + } } } } + log.Debugf("Uploading release asset %s...", assetPathStat.Name()) + assetFile, err := os.Open(pushService.cacheDirectory.AssetPath(release.GetTagName(), assetPathStat.Name())) + if err != nil { + return errors.Wrap(err, "Error opening release asset.") + } + defer assetFile.Close() + progressReader := &ioprogress.Reader{ + Reader: assetFile, + Size: assetPathStat.Size(), + DrawFunc: ioprogress.DrawTerminalf(os.Stderr, ioprogress.DrawTextFormatBytes), + } + if err != nil { + return errors.Wrap(err, "Error opening release asset.") + } + _, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader) + if err == nil { + return nil + } else { + if githubErrorResponse := new(github.ErrorResponse); errors.As(err, &githubErrorResponse) { + for _, innerError := range githubErrorResponse.Errors { + if innerError.Code == "already_exists" { + log.Warn("Asset already existed.") + return nil + } + } + } + if response == nil || response.StatusCode < 500 || attempt >= 5 { + return err + } + log.Warnf("Attempt %d failed to upload release asset (%s), retrying...", attempt, err.Error()) + } } - log.Debugf("Uploading release asset %s...", assetPathStat.Name()) - assetFile, err := os.Open(pushService.cacheDirectory.AssetPath(release.GetTagName(), assetPathStat.Name())) - if err != nil { - return errors.Wrap(err, "Error opening release asset.") - } - defer assetFile.Close() - progressReader := &ioprogress.Reader{ - Reader: assetFile, - Size: assetPathStat.Size(), - DrawFunc: ioprogress.DrawTerminalf(os.Stderr, ioprogress.DrawTextFormatBytes), - } - _, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader) - if err != nil { - return githubapiutil.EnrichResponseError(response, err, "Error uploading release asset.") - } - return nil } func (pushService *pushService) pushReleases() error { From 6bebcc22baa8ce6dc494b68872ca4020de68ba68 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 14 Jun 2022 14:54:49 +0100 Subject: [PATCH 2/2] Split out a method for doing a single upload attempt to prevent resource leaks. --- internal/push/push.go | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/internal/push/push.go b/internal/push/push.go index 130f753..9704421 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -307,6 +307,24 @@ func (pushService *pushService) uploadReleaseAsset(release *github.RepositoryRel return asset, response, nil } +func (pushService *pushService) uploadAsset(release *github.RepositoryRelease, assetPathStat os.FileInfo) (*github.Response, error) { + assetFile, err := os.Open(pushService.cacheDirectory.AssetPath(release.GetTagName(), assetPathStat.Name())) + if err != nil { + return nil, errors.Wrap(err, "Error opening release asset.") + } + defer assetFile.Close() + progressReader := &ioprogress.Reader{ + Reader: assetFile, + Size: assetPathStat.Size(), + DrawFunc: ioprogress.DrawTerminalf(os.Stderr, ioprogress.DrawTextFormatBytes), + } + if err != nil { + return nil, errors.Wrap(err, "Error opening release asset.") + } + _, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader) + return response, err +} + func (pushService *pushService) createOrUpdateReleaseAsset(release *github.RepositoryRelease, existingAssets []*github.ReleaseAsset, assetPathStat os.FileInfo) error { attempt := 0 for { @@ -327,20 +345,7 @@ func (pushService *pushService) createOrUpdateReleaseAsset(release *github.Repos } } log.Debugf("Uploading release asset %s...", assetPathStat.Name()) - assetFile, err := os.Open(pushService.cacheDirectory.AssetPath(release.GetTagName(), assetPathStat.Name())) - if err != nil { - return errors.Wrap(err, "Error opening release asset.") - } - defer assetFile.Close() - progressReader := &ioprogress.Reader{ - Reader: assetFile, - Size: assetPathStat.Size(), - DrawFunc: ioprogress.DrawTerminalf(os.Stderr, ioprogress.DrawTextFormatBytes), - } - if err != nil { - return errors.Wrap(err, "Error opening release asset.") - } - _, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader) + response, err := pushService.uploadAsset(release, assetPathStat) if err == nil { return nil } else {