summaryrefslogtreecommitdiffstats
path: root/services
diff options
context:
space:
mode:
authorPeter Gardfjäll <peter.gardfjall.work@gmail.com>2022-08-29 11:45:20 +0200
committerGitHub <noreply@github.com>2022-08-29 11:45:20 +0200
commit4562d40fcead66e54525f710875377ebf7c4766e (patch)
tree6d77cf4742ce0dd6111a66fd5e4740a0954cce97 /services
parent41c76ad71404bc0b4da83df063e6e10cd4a41e9f (diff)
downloadgitea-4562d40fcead66e54525f710875377ebf7c4766e.tar.gz
gitea-4562d40fcead66e54525f710875377ebf7c4766e.zip
fix hard-coded timeout and error panic in API archive download endpoint (#20925)
* fix hard-coded timeout and error panic in API archive download endpoint This commit updates the `GET /api/v1/repos/{owner}/{repo}/archive/{archive}` endpoint which prior to this PR had a couple of issues. 1. The endpoint had a hard-coded 20s timeout for the archiver to complete after which a 500 (Internal Server Error) was returned to client. For a scripted API client there was no clear way of telling that the operation timed out and that it should retry. 2. Whenever the timeout _did occur_, the code used to panic. This was caused by the API endpoint "delegating" to the same call path as the web, which uses a slightly different way of reporting errors (HTML rather than JSON for example). More specifically, `api/v1/repo/file.go#GetArchive` just called through to `web/repo/repo.go#Download`, which expects the `Context` to have a `Render` field set, but which is `nil` for API calls. Hence, a `nil` pointer error. The code addresses (1) by dropping the hard-coded timeout. Instead, any timeout/cancelation on the incoming `Context` is used. The code addresses (2) by updating the API endpoint to use a separate call path for the API-triggered archive download. This avoids producing HTML-errors on errors (it now produces JSON errors). Signed-off-by: Peter Gardfjäll <peter.gardfjall.work@gmail.com>
Diffstat (limited to 'services')
-rw-r--r--services/repository/archiver/archiver.go71
1 files changed, 61 insertions, 10 deletions
diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go
index ebd3eaf236..ae43503bae 100644
--- a/services/repository/archiver/archiver.go
+++ b/services/repository/archiver/archiver.go
@@ -57,6 +57,21 @@ func (ErrUnknownArchiveFormat) Is(err error) bool {
return ok
}
+// RepoRefNotFoundError is returned when a requested reference (commit, tag) was not found.
+type RepoRefNotFoundError struct {
+ RefName string
+}
+
+// Error implements error.
+func (e RepoRefNotFoundError) Error() string {
+ return fmt.Sprintf("unrecognized repository reference: %s", e.RefName)
+}
+
+func (e RepoRefNotFoundError) Is(err error) bool {
+ _, ok := err.(RepoRefNotFoundError)
+ return ok
+}
+
// NewRequest creates an archival request, based on the URI. The
// resulting ArchiveRequest is suitable for being passed to ArchiveRepository()
// if it's determined that the request still needs to be satisfied.
@@ -103,7 +118,7 @@ func NewRequest(repoID int64, repo *git.Repository, uri string) (*ArchiveRequest
}
}
} else {
- return nil, fmt.Errorf("Unknow ref %s type", r.refName)
+ return nil, RepoRefNotFoundError{RefName: r.refName}
}
return r, nil
@@ -115,6 +130,49 @@ func (aReq *ArchiveRequest) GetArchiveName() string {
return strings.ReplaceAll(aReq.refName, "/", "-") + "." + aReq.Type.String()
}
+// Await awaits the completion of an ArchiveRequest. If the archive has
+// already been prepared the method returns immediately. Otherwise an archiver
+// process will be started and its completion awaited. On success the returned
+// RepoArchiver may be used to download the archive. Note that even if the
+// context is cancelled/times out a started archiver will still continue to run
+// in the background.
+func (aReq *ArchiveRequest) Await(ctx context.Context) (*repo_model.RepoArchiver, error) {
+ archiver, err := repo_model.GetRepoArchiver(ctx, aReq.RepoID, aReq.Type, aReq.CommitID)
+ if err != nil {
+ return nil, fmt.Errorf("models.GetRepoArchiver: %v", err)
+ }
+
+ if archiver != nil && archiver.Status == repo_model.ArchiverReady {
+ // Archive already generated, we're done.
+ return archiver, nil
+ }
+
+ if err := StartArchive(aReq); err != nil {
+ return nil, fmt.Errorf("archiver.StartArchive: %v", err)
+ }
+
+ poll := time.NewTicker(time.Second * 1)
+ defer poll.Stop()
+
+ for {
+ select {
+ case <-graceful.GetManager().HammerContext().Done():
+ // System stopped.
+ return nil, graceful.GetManager().HammerContext().Err()
+ case <-ctx.Done():
+ return nil, ctx.Err()
+ case <-poll.C:
+ archiver, err = repo_model.GetRepoArchiver(ctx, aReq.RepoID, aReq.Type, aReq.CommitID)
+ if err != nil {
+ return nil, fmt.Errorf("repo_model.GetRepoArchiver: %v", err)
+ }
+ if archiver != nil && archiver.Status == repo_model.ArchiverReady {
+ return archiver, nil
+ }
+ }
+ }
+}
+
func doArchive(r *ArchiveRequest) (*repo_model.RepoArchiver, error) {
txCtx, committer, err := db.TxContext()
if err != nil {
@@ -147,11 +205,7 @@ func doArchive(r *ArchiveRequest) (*repo_model.RepoArchiver, error) {
}
}
- rPath, err := archiver.RelativePath()
- if err != nil {
- return nil, err
- }
-
+ rPath := archiver.RelativePath()
_, err = storage.RepoArchives.Stat(rPath)
if err == nil {
if archiver.Status == repo_model.ArchiverGenerating {
@@ -284,13 +338,10 @@ func StartArchive(request *ArchiveRequest) error {
}
func deleteOldRepoArchiver(ctx context.Context, archiver *repo_model.RepoArchiver) error {
- p, err := archiver.RelativePath()
- if err != nil {
- return err
- }
if err := repo_model.DeleteRepoArchiver(ctx, archiver); err != nil {
return err
}
+ p := archiver.RelativePath()
if err := storage.RepoArchives.Delete(p); err != nil {
log.Error("delete repo archive file failed: %v", err)
}