return file object even if there is a problem with the import

api/v1/files.go

@@ -88,32 +88,20 @@ func (api *API) CreateFileForShow(c *gin.Context) {
 		sendError(c, err)
 		return
 	}
-	// TODO: from here on, in case of an error we should either delete the newly created
-	//       file or at least return the id of the created file so the UI can deal with it.
 
+	// 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
 	refID := "" // TODO: get this from query paremeter
 	job, err := api.importer.CreateJob(showID, file.ID, srcURI, sess.Username, refID)
 	if err != nil {
-		// possible errors:
-		//   - any error returned by store.GetFile():
-		//     possible reasons for this:
-		//       * connection problem to the store database
-		//       * somebody deleted the file
-		//   - importer.ErrFileNotNew -> leave file
-		sendError(c, err)
-		return
+		status, _ = statusCodeFromError(err)
+		goto create_file_response
 	}
+
 	if err = job.Start(context.Background(), 3*time.Hour); err != nil { // TODO: hardcoded value
-		// possible errors:
-		//   - importer.ErrAlreadyCanceled -> ??
-		//   - error while creating job workdir -> ??
-		//   - importer.ErrTooManyJobs -> ??
-		sendError(c, err)
-		return
-	}
-	if waitFor == "" {
-		c.JSON(http.StatusCreated, file)
-		return
+		status, _ = statusCodeFromError(err)
+		goto create_file_response
 	}
 
 	switch waitFor {
@@ -122,14 +110,22 @@ func (api *API) CreateFileForShow(c *gin.Context) {
 	case "done":
 		<-job.Done()
 	}
+
+create_file_response:
 	if file, err = api.store.GetFile(showID, file.ID); err != nil {
-		// possible reasons why this failed:
-		//   - connection problem to the store database
-		//   - somebody deleted the file
-		sendError(c, err)
-		return
-	}
-	c.JSON(http.StatusCreated, 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.
+		if err == store.ErrNotFound {
+			sendError(c, err)
+			return
+		}
+		status, _ = statusCodeFromError(err)
+	}
+	c.JSON(status, file)
 }
 
 func (api *API) ReadFileOfShow(c *gin.Context) {

api/v1/utils.go

@@ -38,9 +38,10 @@ func idFromString(s string) (uint64, error) {
 	return strconv.ParseUint(s, 10, 64)
 }
 
-func sendError(c *gin.Context, err error) {
-	code := http.StatusInternalServerError
-	response := ErrorResponse{Error: err.Error()}
+func statusCodeFromError(err error) (code int, response ErrorResponse) {
+	code = http.StatusInternalServerError
+	response = ErrorResponse{Error: err.Error()}
+
 	switch err.(type) {
 	case *store.ErrFileInUse:
 		code = http.StatusConflict
@@ -83,6 +84,11 @@ func sendError(c *gin.Context, err error) {
 			code = http.StatusConflict
 		}
 	}
+	return
+}
+
+func sendError(c *gin.Context, err error) {
+	code, response := statusCodeFromError(err)
 	c.JSON(code, response)
 }