From 3ce15a9eb04b41bcfe5c61a31e3d75101068093d Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Wed, 14 Sep 2016 21:11:51 -0700 Subject: [PATCH] pull: relay along errors encountered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/odeke-em/drive/issues/734. Capture errors after parallelized pulls. Also avoid shadowing errors on result waiting. The changed code was legacy of an oversight after the old days in which we didn't send back exit codes to the outside world. After exit codes were added to drive, the pull code was never touched. Even still though, we had logging for errors and users could see their errors. Exhibits: * Unsuccessful pull ```shell drive pull --export pdf,rtf t1; echo $? Resolving... $ drive pull --export pdf,rtf t1; echo $? Resolving... M /test-docs/t1 Modification count 1 yroceed with the changes? [Y/n]: – /test-docs/t1 err: download: failed for url "". StatusCode: 400 download: failed for url "". StatusCode: 400 255 ``` * Successful pull ```shell $ drive pull --export pdf,rtf t1; echo $? Resolving... M /test-docs/t1 Modification count 1 Proceed with the changes? [Y/n]:y Exported '/Users/emmanuelodeke/emm.odeke@gmail.com/test-docs/t1' to '/Users/emmanuelodeke/emm.odeke@gmail.com/test-docs/t1_exports/t1.rtf' Exported '/Users/emmanuelodeke/emm.odeke@gmail.com/test-docs/t1' to '/Users/emmanuelodeke/emm.odeke@gmail.com/test-docs/t1_exports/t1.pdf' 0 ``` --- src/pull.go | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/pull.go b/src/pull.go index 0068cd7e..31fbbd08 100644 --- a/src/pull.go +++ b/src/pull.go @@ -449,9 +449,10 @@ func (g *Commands) playPullChanges(cl []*Change, exports []string, opMap *map[Op results := semalim.Run(jobsChan, uint64(n)) for result := range results { - res, err := result.Value(), result.Err() - if err != nil { - g.log.LogErrf("pull: %s err: %v\n", res, err) + res, rErr := result.Value(), result.Err() + if rErr != nil { + msg := fmt.Sprintf("%v err: %v\n", res, rErr) + err = reComposeError(err, msg) } } @@ -602,13 +603,17 @@ func (g *Commands) makeExportsDir(segments ...string) string { } func (g *Commands) export(f *File, destAbsPath string, exports []string) (manifest []string, err error) { - if len(exports) < 1 || f == nil { - return + if len(exports) < 1 { + return nil, nil + } + + if f == nil { + return nil, fmt.Errorf("nil file dereference") } dirPath := g.makeExportsDir(destAbsPath) - if err = os.MkdirAll(dirPath, os.ModeDir|0755); err != nil { - return + if err := os.MkdirAll(dirPath, os.ModeDir|0755); err != nil { + return nil, err } var ok bool @@ -631,15 +636,17 @@ func (g *Commands) export(f *File, destAbsPath string, exports []string) (manife } n := len(waitables) - doneAck := make(chan bool, n) + errsChan := make(chan error, n) basePath := filepath.Base(f.Name) baseDir := path.Join(dirPath, basePath) for _, exportee := range waitables { - go func(baseDirPath, id string, urlMExt *urlMimeTypeExt) error { + go func(baseDirPath, id string, urlMExt *urlMimeTypeExt) { + var err error + defer func() { - doneAck <- true + errsChan <- err }() exportPath := sepJoin(".", baseDirPath, urlMExt.ext) @@ -661,20 +668,21 @@ func (g *Commands) export(f *File, destAbsPath string, exports []string) (manife exportURL: urlMExt.url, } - err := g.singleDownload(&dlArg) + err = g.singleDownload(&dlArg) if err == nil { manifest = append(manifest, exportPath) } - - return err }(baseDir, f.Id, exportee) } for i := 0; i < n; i++ { - <-doneAck + ithErr := <-errsChan + if ithErr != nil { + err = reComposeError(err, ithErr.Error()) + } } - return + return manifest, err } func isLocalFile(f *File) bool {