Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

NXCM-4852: EOF on deploy #697

Merged
merged 6 commits into from

3 participants

Tamas Cservenak ifedorenko Benjamin Hanzelmann
Tamas Cservenak
Owner

Introduced new LocalStorage Exception to mark this, and translating it on REST level to some other status that 500 Server Error, that automagically kicks in on those response codes.

cstamas added some commits
Tamas Cservenak cstamas NXCM-4852: Client drops connection during upload.
Introduced new LS exception that denotes EOF situation.
Most likely we really need to get rid of StorageException
altogether.
0e6ef37
Tamas Cservenak cstamas NXCM-4852: Refining response code
EOF while storing into local storage might happen in
two cases (if not counting other -- possible internal bugs):
* client uploads content, and drops connection
* proxied repository drops connection while item is being cached

Response code does not matter in 1st case, as client (who'd receive
the code) did drop the connection anyway, but we have to end the
transaction somehow.
Second case does matters, as in that case, after retries are tried
(LSEx does invoke retries, and if suppose remote peer drops connection
as many times as retries are made), responding with 404 instead
of 400 makes more sense.
2573dc5
Tamas Cservenak cstamas NXCM-4852: Added UT
Ensuring that when input stream of item being stored in repository
throws EOF exception (premature end of file), a newly introduced
LocalStorage exception is thrown. Distinction is needed, to handle
this case differently on REST level than all the other "local storage"
exceptions that are basically all 500 "server errors".
f81e211
ifedorenko

-1

Need to verify partially downloaded files are not left behind.

cstamas added some commits
Tamas Cservenak cstamas NXCM-4852: UT extended
UT is now checking not only the exception being
thrown when EOF occurs during storing, but also
that there are no remnant files left in storage.
e94aaf4
Tamas Cservenak cstamas NXCM-4852: UT fix
Added check for tmp files too, and changed when EOF is
thrown to ensure we got _some_ content.
5bdcb09
Tamas Cservenak cstamas NXCM-4852: Explained response code 4c66cb7
Tamas Cservenak
Owner

CI +1'd

Benjamin Hanzelmann

+1

Tamas Cservenak cstamas merged commit 742de42 into from
David Z. Chen davidzchen referenced this pull request from a commit
Tamas Cservenak cstamas Proxy EOF detection.
Issue NXCM-4864 revealed that pull request
#697
is insufficient to handle case when proxy remote peer
drops connection unexpectedly.

NXCM-4864 reveals what exact exception is thrown by HC4
when "remote peer" drops connection unexpectedly. Inspecting
the exception also reveals it is not an `java.io.EOFException`
descendant, but descends directly from `java.io.IOException`.
Hence, changes in pull request
#697
(that kicks in on EOFException) will never kick in (and
this case will be handled as "generic IO problem" and will
auto block remote peer too).

So, we introduced a "wrapper" around the HC4 stream, to
have it "translate" HC4 specific exception
into one that is recognised (and handled) by Nexus Core.
Moreover, as it is highly likely that other transports will
use other exceptions (descending or not from EOFException),
using this technique we could always ensure that this case is
properly handled by Nexus Core.

Here, newly introduced Hc4InputStream is meant to translate
the `org.apache.http.ConnectionClosedException` thrown by
`org.apache.http.impl.io.ContentLengthInputStream` read methods.
e82b00a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 7, 2012
  1. Tamas Cservenak

    NXCM-4852: Client drops connection during upload.

    cstamas authored
    Introduced new LS exception that denotes EOF situation.
    Most likely we really need to get rid of StorageException
    altogether.
  2. Tamas Cservenak

    NXCM-4852: Refining response code

    cstamas authored
    EOF while storing into local storage might happen in
    two cases (if not counting other -- possible internal bugs):
    * client uploads content, and drops connection
    * proxied repository drops connection while item is being cached
    
    Response code does not matter in 1st case, as client (who'd receive
    the code) did drop the connection anyway, but we have to end the
    transaction somehow.
    Second case does matters, as in that case, after retries are tried
    (LSEx does invoke retries, and if suppose remote peer drops connection
    as many times as retries are made), responding with 404 instead
    of 400 makes more sense.
  3. Tamas Cservenak

    NXCM-4852: Added UT

    cstamas authored
    Ensuring that when input stream of item being stored in repository
    throws EOF exception (premature end of file), a newly introduced
    LocalStorage exception is thrown. Distinction is needed, to handle
    this case differently on REST level than all the other "local storage"
    exceptions that are basically all 500 "server errors".
Commits on Dec 10, 2012
  1. Tamas Cservenak

    NXCM-4852: UT extended

    cstamas authored
    UT is now checking not only the exception being
    thrown when EOF occurs during storing, but also
    that there are no remnant files left in storage.
  2. Tamas Cservenak

    NXCM-4852: UT fix

    cstamas authored
    Added check for tmp files too, and changed when EOF is
    thrown to ensure we got _some_ content.
Commits on Dec 11, 2012
  1. Tamas Cservenak
This page is out of date. Refresh to see the latest.
40 nexus/nexus-api/src/main/java/org/sonatype/nexus/proxy/LocalStorageEofException.java
View
@@ -0,0 +1,40 @@
+/*
+ * Sonatype Nexus (TM) Open Source Version
+ * Copyright (c) 2007-2012 Sonatype, Inc.
+ * All rights reserved. Includes the third-party code listed at http://links.sonatype.com/products/nexus/oss/attributions.
+ *
+ * This program and the accompanying materials are made available under the terms of the Eclipse Public License Version 1.0,
+ * which accompanies this distribution and is available at http://www.eclipse.org/legal/epl-v10.html.
+ *
+ * Sonatype Nexus (TM) Professional Version is available from Sonatype, Inc. "Sonatype" and "Sonatype Nexus" are trademarks
+ * of Sonatype, Inc. Apache Maven is a trademark of the Apache Software Foundation. M2eclipse is a trademark of the
+ * Eclipse Foundation. All other trademarks are the property of their respective owners.
+ */
+package org.sonatype.nexus.proxy;
+
+/**
+ * Local storage exception thrown by local storage when stream/content being pushed (stored) ends prematurely (EOFs).
+ * Denotes an unrecoverable state, but is not resolvable by Nexus core (happens when client drops connection
+ * during upload, recovery is to have client retry upload).
+ *
+ * @author cstamas
+ * @since 2.3
+ */
+public class LocalStorageEofException
+ extends LocalStorageException
+{
+ public LocalStorageEofException( String msg )
+ {
+ super( msg );
+ }
+
+ public LocalStorageEofException( String msg, Throwable cause )
+ {
+ super( msg, cause );
+ }
+
+ public LocalStorageEofException( Throwable cause )
+ {
+ super( cause.getMessage(), cause );
+ }
+}
15 nexus/nexus-proxy/src/main/java/org/sonatype/nexus/proxy/storage/local/fs/DefaultFSPeer.java
View
@@ -14,12 +14,12 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import java.io.EOFException;
import java.io.File;
import java.io.FileFilter;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.net.MalformedURLException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -31,11 +31,11 @@
import org.codehaus.plexus.util.IOUtil;
import org.sonatype.nexus.logging.AbstractLoggingComponent;
import org.sonatype.nexus.proxy.ItemNotFoundException;
+import org.sonatype.nexus.proxy.LocalStorageEofException;
import org.sonatype.nexus.proxy.LocalStorageException;
import org.sonatype.nexus.proxy.ResourceStoreRequest;
import org.sonatype.nexus.proxy.access.Action;
import org.sonatype.nexus.proxy.item.ContentLocator;
-import org.sonatype.nexus.proxy.item.RepositoryItemUid;
import org.sonatype.nexus.proxy.item.RepositoryItemUidLock;
import org.sonatype.nexus.proxy.item.StorageItem;
import org.sonatype.nexus.proxy.item.uid.IsItemAttributeMetacontentAttribute;
@@ -113,6 +113,17 @@ public void storeItem( final Repository repository, final File repositoryBaseDir
os.flush();
}
+ catch ( EOFException e )
+ {
+ if ( hiddenTarget != null )
+ {
+ hiddenTarget.delete();
+ }
+
+ throw new LocalStorageEofException( String.format(
+ "EOF during storing on path \"%s\" (while writing to hiddenTarget: \"%s\")",
+ item.getRepositoryItemUid().toString(), hiddenTarget.getAbsolutePath() ), e );
+ }
catch ( IOException e )
{
if ( hiddenTarget != null )
69 nexus/nexus-proxy/src/test/java/org/sonatype/nexus/proxy/SimplePullTest.java
View
@@ -14,8 +14,14 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.emptyIterableOf;
import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import java.io.ByteArrayInputStream;
+import java.io.EOFException;
+import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
@@ -41,6 +47,7 @@
import org.sonatype.nexus.proxy.repository.GroupRepository;
import org.sonatype.nexus.proxy.repository.LocalStatus;
import org.sonatype.nexus.proxy.repository.Repository;
+import org.sonatype.nexus.util.WrappingInputStream;
import com.google.common.base.Strings;
@@ -442,6 +449,68 @@ public void testNexus4985GroupsShouldNotSwallowMemberExceptions()
}
}
+ /**
+ * NXCM-4582: When Local storage is about to store something, but during "store" operation source stream EOFs,
+ * the new LocalStorage exception should be thrown, to differentiate from other "fatal" (like disk full or what not)
+ * error.
+ */
+ @Test
+ public void testNXCM4852()
+ throws Exception
+ {
+ final Repository repository = getRepositoryRegistry().getRepository( "inhouse" );
+ ResourceStoreRequest request =
+ new ResourceStoreRequest( "/activemq/activemq-core/1.2/activemq-core-1.2.jar", true );
+
+ try
+ {
+ repository.storeItem( request, new WrappingInputStream(
+ new ByteArrayInputStream( "123456789012345678901234567890".getBytes() ) )
+ {
+ @Override
+ public int read()
+ throws IOException
+ {
+ int result = super.read();
+ if ( result == -1 )
+ {
+ throw new EOFException( "Foo" );
+ }
+ else
+ {
+ return result;
+ }
+ }
+
+ @Override
+ public int read( final byte[] b, final int off, final int len )
+ throws IOException
+ {
+ int result = super.read( b, off, len );
+ if ( result == -1 )
+ {
+ throw new EOFException( "Foo" );
+ }
+ return result;
+ }
+ }, null );
+
+ Assert.fail( "We expected a LocalStorageEofException to be thrown" );
+ }
+ catch ( LocalStorageEofException e )
+ {
+ // good, we expected this
+ }
+ finally
+ {
+ // now we have to ensure no remnant files exists
+ assertThat( repository.getLocalStorage().containsItem( repository, request ), is( false ) );
+ // no tmp files should exists either
+ assertThat( repository.getLocalStorage().listItems( repository, new ResourceStoreRequest( "/.nexus/tmp" ) ), is( empty() ) );
+ }
+ }
+
+
//
protected int countOccurence( final String string, final String snippet )
9 ...s-restlet1x-plugin/src/main/java/org/sonatype/nexus/rest/AbstractResourceStoreContentPlexusResource.java
View
@@ -50,6 +50,7 @@
import org.sonatype.nexus.proxy.IllegalOperationException;
import org.sonatype.nexus.proxy.IllegalRequestException;
import org.sonatype.nexus.proxy.ItemNotFoundException;
+import org.sonatype.nexus.proxy.LocalStorageEofException;
import org.sonatype.nexus.proxy.NoSuchRepositoryException;
import org.sonatype.nexus.proxy.NoSuchResourceStoreException;
import org.sonatype.nexus.proxy.RemoteStorageTransportOverloadedException;
@@ -881,6 +882,14 @@ protected void handleException( final Request req, final Response res, final Exc
{
throw (ResourceException) t;
}
+ else if ( t instanceof LocalStorageEofException )
+ {
+ // in case client drops connection, this makes not much sense, as he will not
+ // receive this response, but we have to end it somehow.
+ // but, in case when remote proxy peer drops connection on us regularly
+ // this makes sense
+ throw new ResourceException( getStatus( Status.CLIENT_ERROR_NOT_FOUND, t ), t );
+ }
else if ( t instanceof IllegalArgumentException )
{
throw new ResourceException( getStatus( Status.CLIENT_ERROR_BAD_REQUEST, t ), t );
Something went wrong with that request. Please try again.