Merge branch 'topic/create-file-error'

api/v1/files.go

@@ -90,17 +90,14 @@ func (api *API) CreateFileForShow(c *gin.Context) {
 	}
 
 	// From here on, in case of an error we should either delete the newly created
-	// file or at least return the id of it so the API user can deal with the problem.
-	status := http.StatusCreated
+	// file or return the file object as ErrorResponse.Detail so the API user can deal with the problem.
 	refID := "" // TODO: get this from query paremeter
 	job, err := api.importer.CreateJob(showID, file.ID, srcURI, sess.Username, refID)
 	if err != nil {
-		status, _ = statusCodeFromError(err)
 		goto create_file_response
 	}
 
 	if err = job.Start(context.Background(), 3*time.Hour); err != nil { // TODO: hardcoded value
-		status, _ = statusCodeFromError(err)
 		goto create_file_response
 	}
 
@@ -112,20 +109,32 @@ func (api *API) CreateFileForShow(c *gin.Context) {
 	}
 
 create_file_response:
-	if file, err = api.store.GetFile(showID, file.ID); err != nil {
-		// GetFile only fails if the file does not exist or there is
-		// a problem with the database connection. The former probably
-		// means that the file got deleted by somebody else... all other
-		// errors most likely mean that the file still exists. In this
-		// case it's better to return the file so the API user can use
-		// the file ID for further inspection.
-		if err == store.ErrNotFound {
-			sendError(c, err)
-			return
-		}
-		status, _ = statusCodeFromError(err)
-	}
-	c.JSON(status, file)
+	// GetFile only fails if the file does not exist or there is
+	// a problem with the database connection. The former probably
+	// means that the file got deleted by somebody else... all other
+	// errors most likely mean that the file still exists. In this
+	// case it's better to return the file so the API user can use
+	// the file ID for further inspection.
+	getFile, getErr := api.store.GetFile(showID, file.ID)
+	if getErr == store.ErrNotFound {
+		sendError(c, ErrFileVanished)
+		return
+	}
+	// don't mask the original error
+	if err == nil {
+		err = getErr
+	}
+	if getFile != nil {
+		file = getFile
+	}
+
+	if err == nil {
+		c.JSON(http.StatusCreated, file)
+		return
+	}
+	status, errResp := statusCodeFromError(err)
+	errResp.Detail = file
+	c.JSON(status, errResp)
 }
 
 func (api *API) ReadFileOfShow(c *gin.Context) {

api/v1/types.go

@@ -35,11 +35,12 @@ import (
 var (
 	ErrNotFlowJSUpload            = errors.New("this is not a flow.js upload")
 	ErrFlowJSChunkAlreadUploading = errors.New("chunk is already uploading or done")
+	ErrFileVanished               = errors.New("the file vanished from the store while preparing the import")
 )
 
 type ErrorResponse struct {
-	Error   string      `json:"error,omitempty"`
-	Details interface{} `json:"details,omitempty"`
+	Error  string      `json:"error,omitempty"`
+	Detail interface{} `json:"detail,omitempty"`
 }
 
 // Shows

api/v1/utils.go

@@ -45,15 +45,19 @@ func statusCodeFromError(err error) (code int, response ErrorResponse) {
 	switch err.(type) {
 	case *store.ErrFileInUse:
 		code = http.StatusConflict
-		response.Details = err
+		response.Detail = err
 	case store.ErrInvalidMetadataField:
 		code = http.StatusBadRequest
 	case *importer.JobSourceResult:
-		response.Details = err.(*importer.JobSourceResult).Log
+		response.Detail = err.(*importer.JobSourceResult).Log
 	default:
 		switch err {
 		case ErrNotFlowJSUpload:
 			code = http.StatusConflict
+		case ErrFlowJSChunkAlreadUploading:
+			code = http.StatusConflict
+		case ErrFileVanished:
+			code = http.StatusConflict
 
 		case store.ErrNotImplemented:
 			code = http.StatusNotImplemented