Better fix #1062 (return error instead of panic)

This commit is contained in:
Mike Farah 2022-01-14 15:40:20 +11:00
parent 78b45a3eb0
commit 606ef91cc6
4 changed files with 22 additions and 13 deletions

View File

@ -39,7 +39,7 @@ Note that it consumes more memory than eval.
} }
return cmdEvalAll return cmdEvalAll
} }
func evaluateAll(cmd *cobra.Command, args []string) error { func evaluateAll(cmd *cobra.Command, args []string) (cmdError error) {
// 0 args, read std in // 0 args, read std in
// 1 arg, null input, process expression // 1 arg, null input, process expression
// 1 arg, read file in sequence // 1 arg, read file in sequence
@ -67,7 +67,11 @@ func evaluateAll(cmd *cobra.Command, args []string) error {
} }
// need to indirectly call the function so that completedSuccessfully is // need to indirectly call the function so that completedSuccessfully is
// passed when we finish execution as opposed to now // passed when we finish execution as opposed to now
defer func() { writeInPlaceHandler.FinishWriteInPlace(completedSuccessfully) }() defer func() {
if cmdError == nil {
cmdError = writeInPlaceHandler.FinishWriteInPlace(completedSuccessfully)
}
}()
} }
format, err := yqlib.OutputFormatFromString(outputFormat) format, err := yqlib.OutputFormatFromString(outputFormat)

View File

@ -53,7 +53,7 @@ func processExpression(expression string) string {
return expression return expression
} }
func evaluateSequence(cmd *cobra.Command, args []string) error { func evaluateSequence(cmd *cobra.Command, args []string) (cmdError error) {
// 0 args, read std in // 0 args, read std in
// 1 arg, null input, process expression // 1 arg, null input, process expression
// 1 arg, read file in sequence // 1 arg, read file in sequence
@ -80,7 +80,11 @@ func evaluateSequence(cmd *cobra.Command, args []string) error {
} }
// need to indirectly call the function so that completedSuccessfully is // need to indirectly call the function so that completedSuccessfully is
// passed when we finish execution as opposed to now // passed when we finish execution as opposed to now
defer func() { writeInPlaceHandler.FinishWriteInPlace(completedSuccessfully) }() defer func() {
if cmdError == nil {
cmdError = writeInPlaceHandler.FinishWriteInPlace(completedSuccessfully)
}
}()
} }
format, err := yqlib.OutputFormatFromString(outputFormat) format, err := yqlib.OutputFormatFromString(outputFormat)

View File

@ -7,7 +7,7 @@ import (
"os" "os"
) )
func tryRenameFile(from string, to string) { func tryRenameFile(from string, to string) error {
if renameError := os.Rename(from, to); renameError != nil { if renameError := os.Rename(from, to); renameError != nil {
log.Debugf("Error renaming from %v to %v, attempting to copy contents", from, to) log.Debugf("Error renaming from %v to %v, attempting to copy contents", from, to)
log.Debug(renameError.Error()) log.Debug(renameError.Error())
@ -15,11 +15,11 @@ func tryRenameFile(from string, to string) {
// can't do this rename when running in docker to a file targeted in a mounted volume, // can't do this rename when running in docker to a file targeted in a mounted volume,
// so gracefully degrade to copying the entire contents. // so gracefully degrade to copying the entire contents.
if copyError := copyFileContents(from, to); copyError != nil { if copyError := copyFileContents(from, to); copyError != nil {
panic(fmt.Errorf("failed copying from %v to %v: %w", from, to, copyError)) return fmt.Errorf("failed copying from %v to %v: %w", from, to, copyError)
} else {
tryRemoveTempFile(from)
} }
tryRemoveTempFile(from)
} }
return nil
} }
func tryRemoveTempFile(filename string) { func tryRemoveTempFile(filename string) {

View File

@ -6,7 +6,7 @@ import (
type writeInPlaceHandler interface { type writeInPlaceHandler interface {
CreateTempFile() (*os.File, error) CreateTempFile() (*os.File, error)
FinishWriteInPlace(evaluatedSuccessfully bool) FinishWriteInPlace(evaluatedSuccessfully bool) error
} }
type writeInPlaceHandlerImpl struct { type writeInPlaceHandlerImpl struct {
@ -39,13 +39,14 @@ func (w *writeInPlaceHandlerImpl) CreateTempFile() (*os.File, error) {
return file, err return file, err
} }
func (w *writeInPlaceHandlerImpl) FinishWriteInPlace(evaluatedSuccessfully bool) { func (w *writeInPlaceHandlerImpl) FinishWriteInPlace(evaluatedSuccessfully bool) error {
log.Debug("Going to write-inplace, evaluatedSuccessfully=%v, target=%v", evaluatedSuccessfully, w.inputFilename) log.Debug("Going to write-inplace, evaluatedSuccessfully=%v, target=%v", evaluatedSuccessfully, w.inputFilename)
safelyCloseFile(w.tempFile) safelyCloseFile(w.tempFile)
if evaluatedSuccessfully { if evaluatedSuccessfully {
log.Debug("Moving temp file to target") log.Debug("Moving temp file to target")
tryRenameFile(w.tempFile.Name(), w.inputFilename) return tryRenameFile(w.tempFile.Name(), w.inputFilename)
} else {
tryRemoveTempFile(w.tempFile.Name())
} }
tryRemoveTempFile(w.tempFile.Name())
return nil
} }