From 25455a21301fbf21bc4db05d1f60787d87f12095 Mon Sep 17 00:00:00 2001 From: Damian Jansen Date: Thu, 5 Apr 2018 14:41:00 +1000 Subject: [PATCH] fix(ZNTA-568): Provide upload failure details to the user Where possible, pass the exception reason to the user so it is easier to debug a failed upload. --- .../java/org/zanata/adapter/po/PoReader2.java | 4 +-- .../zanata/adapter/OkapiFilterAdapter.java | 5 ++-- .../org/zanata/file/SourceDocumentUpload.java | 7 +++--- .../zanata/rest/service/ResourceUtils.java | 2 +- .../impl/TranslationFileServiceImpl.java | 25 ++++++++++++------- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/common/zanata-adapter-po/src/main/java/org/zanata/adapter/po/PoReader2.java b/common/zanata-adapter-po/src/main/java/org/zanata/adapter/po/PoReader2.java index 8f087deaf8..3536006ebb 100644 --- a/common/zanata-adapter-po/src/main/java/org/zanata/adapter/po/PoReader2.java +++ b/common/zanata-adapter-po/src/main/java/org/zanata/adapter/po/PoReader2.java @@ -96,7 +96,7 @@ public TranslationsResource extractTarget(InputSource inputSource) { if (message.isHeader()) { if (headerFound) - throw new IllegalStateException("found a second header!"); + throw new IllegalStateException("multiple headers found"); headerFound = true; // add target header data @@ -208,7 +208,7 @@ public Resource extractTemplate(InputSource inputSource, if (message.isHeader()) { if (headerFound) - throw new IllegalStateException("found a second header!"); + throw new IllegalStateException("multiple headers found"); headerFound = true; // store POT data diff --git a/server/services/src/main/java/org/zanata/adapter/OkapiFilterAdapter.java b/server/services/src/main/java/org/zanata/adapter/OkapiFilterAdapter.java index 28efc0fb34..432ac30143 100644 --- a/server/services/src/main/java/org/zanata/adapter/OkapiFilterAdapter.java +++ b/server/services/src/main/java/org/zanata/adapter/OkapiFilterAdapter.java @@ -31,6 +31,7 @@ import net.sf.okapi.common.Event; import net.sf.okapi.common.EventType; import net.sf.okapi.common.IParameters; +import net.sf.okapi.common.exceptions.OkapiException; import net.sf.okapi.common.exceptions.OkapiIOException; import net.sf.okapi.common.filters.IFilter; import net.sf.okapi.common.filterwriter.GenericContent; @@ -197,7 +198,7 @@ public Resource parseDocumentFile(@Nonnull URI documentContent, } } } - } catch (OkapiIOException e) { + } catch (OkapiException e) { throw new FileFormatAdapterException("Unable to parse document", e); } finally { filter.close(); @@ -332,7 +333,7 @@ protected TranslationsResource parseTranslationFile(RawDocument rawDoc, } } } - } catch (OkapiIOException e) { + } catch (OkapiException e) { throw new FileFormatAdapterException( "Unable to parse translation file", e); } finally { diff --git a/server/services/src/main/java/org/zanata/file/SourceDocumentUpload.java b/server/services/src/main/java/org/zanata/file/SourceDocumentUpload.java index 8fefae24e2..4d49c07955 100644 --- a/server/services/src/main/java/org/zanata/file/SourceDocumentUpload.java +++ b/server/services/src/main/java/org/zanata/file/SourceDocumentUpload.java @@ -28,6 +28,7 @@ import java.io.Serializable; import javax.annotation.Nonnull; import javax.enterprise.context.Dependent; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import com.google.common.collect.Sets; @@ -282,12 +283,12 @@ private void processAdapterFile(@Nonnull File tempFile, GlobalDocumentId id, id.getVersionSlug(), doc, Sets.newHashSet(PotEntryHeader.ID, SimpleComment.ID), false); - } catch (SecurityException e) { + } catch (SecurityException | ZanataServiceException e) { throw new DocumentUploadException(Status.INTERNAL_SERVER_ERROR, e.getMessage(), e); - } catch (ZanataServiceException e) { + } catch (WebApplicationException e) { throw new DocumentUploadException(Status.INTERNAL_SERVER_ERROR, - e.getMessage(), e); + e.getResponse().getEntity().toString(), e); } String contentHash = uploadForm.getHash(); DocumentType documentType = diff --git a/server/services/src/main/java/org/zanata/rest/service/ResourceUtils.java b/server/services/src/main/java/org/zanata/rest/service/ResourceUtils.java index 8f263a0290..6cb2122867 100644 --- a/server/services/src/main/java/org/zanata/rest/service/ResourceUtils.java +++ b/server/services/src/main/java/org/zanata/rest/service/ResourceUtils.java @@ -164,7 +164,7 @@ boolean transferFromTextFlows(List from, HDocument to, for (TextFlow tf : from) { if (!incomingIds.add(tf.getId())) { Response response = Response.status(Status.BAD_REQUEST).entity( - "encountered TextFlow with duplicate ID " + tf.getId()) + "Encountered TextFlow with duplicate ID") .build(); log.warn( "encountered TextFlow with duplicate ID {}", tf.getId()); diff --git a/server/services/src/main/java/org/zanata/service/impl/TranslationFileServiceImpl.java b/server/services/src/main/java/org/zanata/service/impl/TranslationFileServiceImpl.java index e023dc1d47..c54c26f612 100644 --- a/server/services/src/main/java/org/zanata/service/impl/TranslationFileServiceImpl.java +++ b/server/services/src/main/java/org/zanata/service/impl/TranslationFileServiceImpl.java @@ -21,6 +21,7 @@ package org.zanata.service.impl; import com.google.common.base.Optional; +import com.google.common.base.Throwables; import com.google.common.collect.MapMaker; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; @@ -182,11 +183,11 @@ public TranslationsResource parseAdapterTranslationFile(File tempFile, try { transRes = adapter.parseTranslationFile(tempFile.toURI(), doc.getSourceLocaleId(), localeId, getAdapterParams(doc)); - } catch (FileFormatAdapterException e) { - throw new ZanataServiceException( - "Error parsing translation file: " + fileName, e); } catch (RuntimeException e) { - throw new ZanataServiceException(e); + Throwable rootCause = Throwables.getRootCause(e); + throw new ZanataServiceException( + "Translation file error: " + fileName + " " + + rootCause.getMessage(), e); } return transRes; } @@ -250,9 +251,12 @@ public Resource parseUpdatedAdapterDocumentFile(URI documentFile, try { doc = adapter.parseDocumentFile(documentFile, new LocaleId("en"), params); - } catch (FileFormatAdapterException e) { + } catch (RuntimeException e) { + Throwable rootCause = Throwables.getRootCause(e); + // Filename may not be a 'file name' e.g GETTEXT throw new ZanataServiceException( - "Error parsing document file: " + fileName, e); + "Error parsing document file: " + fileName + " " + + rootCause.getMessage(), e); } doc.setName(docId); return doc; @@ -268,9 +272,12 @@ private Resource parsePotFile(InputStream fileContents, String docId, boolean offlinePo) { PoReader2 poReader = new PoReader2(offlinePo); // assume english as source locale - Resource res = poReader.extractTemplate(new InputSource(fileContents), - new LocaleId("en"), docId); - return res; + try { + return poReader.extractTemplate(new InputSource(fileContents), + new LocaleId("en"), docId); + } catch (IllegalStateException e) { + throw new ZanataServiceException(e.getMessage()); + } } // TODO replace these with values from DocumentType