Revise file/attachment exception handling (#6949)

Author: labkey-nicka

api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java

@@ -943,7 +943,7 @@ else if (o instanceof MvFieldWrapper mvWrapper)
                             }
                             catch (ConvertHelper.FileConversionException e)
                             {
-                                throw new ApiUsageException(e);
+                                errors.add(new PropertyValidationError(e.getMessage(), pd.getName()));
                             }
                         }
                     }

api/src/org/labkey/api/assay/DefaultAssayRunCreator.java

@@ -482,23 +482,25 @@ public ExpExperiment saveExperimentRun(
 
             return batch;
         }
-        catch (ExperimentException | IOException | ConvertHelper.FileConversionException e)
+        catch (ExperimentException | IOException | ConvertHelper.FileConversionException | BatchValidationException e)
         {
+            // TODO: This is better done as a post-rollback task on the transaction
             // clean up the run results file dir here if it was created, for non-async imports
             AssayResultsFileWriter<?> resultsFileWriter = new AssayResultsFileWriter<>(context.getProtocol(), run, null);
             resultsFileWriter.cleanupPostedFiles(context.getContainer(), false);
 
             cleanPrimaryFile(context);
 
-            if (e instanceof ExperimentException)
-                throw (ExperimentException)e;
-            else if (e instanceof ConvertHelper.FileConversionException)
-                throw new ApiUsageException(e.getMessage(), e);
-            else
-                throw new ExperimentException(e);
-        }
-        catch (BatchValidationException e)
-        {
+            if (e instanceof ExperimentException ee)
+                throw ee;
+
+            // HACK: Rethrowing these as ApiUsageException avoids any upstream consequences of wrapping them in ExperimentException.
+            // Namely, that they are logged to the server/mothership. There has to be a better way.
+            if (e instanceof ConvertHelper.FileConversionException fce)
+                throw new ApiUsageException(fce.getMessage(), fce);
+            else if (e instanceof BatchValidationException bve)
+                throw new ApiUsageException(bve.getMessage(), bve);
+
             throw new ExperimentException(e);
         }
     }

api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java

@@ -43,10 +43,6 @@
 import java.io.InputStream;
 import java.util.ArrayList;
 
-/**
- * Moved from ListQueryUpdateService.java
- * by iansigmon on 2/16/16.
- */
 public class AttachmentDataIterator extends WrapperDataIterator
 {
     final VirtualFile attachmentDir;
@@ -92,19 +88,21 @@ public boolean next() throws BatchValidationException
             for (_AttachmentUploadHelper p : attachmentColumns)
             {
                 Object attachmentValue = get(p.index);
+                if (null == attachmentValue)
+                    continue;
+
                 String filename;
                 AttachmentFile attachmentFile;
 
-                if (null == attachmentValue)
-                    continue;
-                else if (attachmentValue instanceof String str)
+                if (attachmentValue instanceof String str)
                 {
                     if (null == attachmentDir)
                     {
-                        errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload '" + str + "' to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + "."));
+                        errors.addRowError(propertyValidationException(p.domainProperty, attachmentValue));
                         return false;
                     }
-                    filename = (String) attachmentValue;
+
+                    filename = str;
                     InputStream aIS = attachmentDir.getDir(p.domainProperty.getName()).getInputStream(p.uniquifier.uniquify(filename));
                     if (aIS == null)
                     {
@@ -113,25 +111,25 @@ else if (attachmentValue instanceof String str)
                     }
                     attachmentFile = new InputStreamAttachmentFile(aIS, filename);
                 }
-                else if (attachmentValue instanceof AttachmentFile)
+                else if (attachmentValue instanceof AttachmentFile file)
                 {
-                    attachmentFile = (AttachmentFile) attachmentValue;
+                    attachmentFile = file;
                     filename = attachmentFile.getFilename();
                 }
-                else if (attachmentValue instanceof File)
+                else if (attachmentValue instanceof File file)
                 {
-                    attachmentFile = new FileAttachmentFile((File) attachmentValue);
+                    attachmentFile = new FileAttachmentFile(file);
                     filename = attachmentFile.getFilename();
                 }
                 else
                 {
-                    errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file."));
+                    errors.addRowError(propertyValidationException(p.domainProperty, attachmentValue));
                     return false;
                 }
 
                 if (entityIdIndex == 0)
                 {
-                    errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file."));
+                    errors.addRowError(rowValidationException("Unable to create attachment file."));
                     return false;
                 }
 
@@ -172,6 +170,16 @@ else if (attachmentValue instanceof File)
         }
     }
 
+    private ValidationException propertyValidationException(DomainProperty property, Object value)
+    {
+        return rowValidationException(String.format("Can't upload '%s' to field %s with type %s.", value, property.getName(), property.getType().getLabel()));
+    }
+
+    private ValidationException rowValidationException(String message)
+    {
+        return new ValidationException("Row " + get(0) + ": " + message);
+    }
+
     public static DataIteratorBuilder getAttachmentDataIteratorBuilder(TableInfo ti, @NotNull final DataIteratorBuilder builder,
                                                                        final User user,
                                                                        @Nullable final VirtualFile attachmentDir,