Skip to content

Commit

Permalink
Improvements for ContentCodec exceptions (#1428)
Browse files Browse the repository at this point in the history
Motivation:

Align the order or arguments with standard JDK classes and throw these
types in all cases.

Modification:

- Swap `message` and `cause` arguments order to make them similar to the
superclass;
- Throw `CodecDecodingException/CodecEncodingException` when the codec is
closed;
- Remove unnecessary `requireNonNull` for aggregated encode and decode;

Result:

Aligned exceptions API and consistent exception type thrown from the codec.
  • Loading branch information
idelpivnitskiy authored Mar 11, 2021
1 parent a72066d commit 1a7e144
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,17 @@ public CodecDecodingException(final ContentCodec codec, final String message) {
* New instance.
*
* @param codec the codec in use.
* @param cause the cause of the exception.
* @param message the reason of this exception.
* @param cause the cause of the exception.
*/
public CodecDecodingException(final ContentCodec codec, final Throwable cause, final String message) {
public CodecDecodingException(final ContentCodec codec, final String message, final Throwable cause) {
super(message, cause);
this.codec = codec;
}

/**
* Returns the codec in use when this exception occurred.
*
* @return the codec in use when this exception occurred.
*/
public ContentCodec codec() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,17 @@ public CodecEncodingException(final ContentCodec codec, final String message) {
* New instance.
*
* @param codec the codec in use.
* @param cause the cause of the exception.
* @param message the reason of this exception.
* @param cause the cause of the exception.
*/
public CodecEncodingException(final ContentCodec codec, final Throwable cause, final String message) {
public CodecEncodingException(final ContentCodec codec, final String message, final Throwable cause) {
super(message, cause);
this.codec = requireNonNull(codec);
}

/**
* Returns the codec in use when this exception occurred.
*
* @return the codec in use when this exception occurred.
*/
public ContentCodec codec() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.servicetalk.concurrent.api.Publisher;
import io.servicetalk.encoding.api.CodecDecodingException;
import io.servicetalk.encoding.api.CodecEncodingException;
import io.servicetalk.encoding.api.ContentCodec;

import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandler;
Expand Down Expand Up @@ -63,7 +64,6 @@ final class NettyChannelContentCodec extends AbstractContentCodec {

@Override
public Buffer encode(final Buffer src, final int offset, final int length, final BufferAllocator allocator) {
requireNonNull(src);
requireNonNull(allocator);

final Buffer slice = src.slice(src.readerIndex() + offset, length);
Expand Down Expand Up @@ -93,7 +93,7 @@ public Buffer encode(final Buffer src, final int offset, final int length, final
} catch (CodecEncodingException e) {
throw e;
} catch (Throwable e) {
throw new CodecEncodingException(this, e, "");
throw wrapEncodingException(this, e);
} finally {
safeCleanup(channel);
}
Expand Down Expand Up @@ -153,7 +153,7 @@ public void onNext(Buffer next) {
subscription.request(1);
}
} catch (Throwable t) {
throw new CodecEncodingException(NettyChannelContentCodec.this, t, "");
throw wrapEncodingException(NettyChannelContentCodec.this, t);
}
}

Expand All @@ -168,7 +168,7 @@ public void onComplete() {
try {
cleanup(channel);
} catch (Throwable t) {
subscriber.onError(new CodecEncodingException(NettyChannelContentCodec.this, t, ""));
subscriber.onError(wrapEncodingException(NettyChannelContentCodec.this, t));
return;
}

Expand All @@ -179,7 +179,6 @@ public void onComplete() {

@Override
public Buffer decode(final Buffer src, final int offset, final int length, final BufferAllocator allocator) {
requireNonNull(src);
requireNonNull(allocator);

final Buffer slice = src.slice(src.readerIndex() + offset, length);
Expand Down Expand Up @@ -207,7 +206,7 @@ public Buffer decode(final Buffer src, final int offset, final int length, final
} catch (CodecDecodingException e) {
throw e;
} catch (Throwable e) {
throw new CodecDecodingException(this, e, "");
throw wrapDecodingException(this, e);
} finally {
safeCleanup(channel);
}
Expand Down Expand Up @@ -235,7 +234,8 @@ public void onSubscribe(final PublisherSource.Subscription subscription) {
public void onNext(@Nullable final Buffer src) {
assert subscription != null;
if (!channel.isOpen()) {
throw new IllegalStateException("Stream decoder previously closed but more input arrived");
throw new CodecDecodingException(NettyChannelContentCodec.this,
"Stream decoder previously closed but more input arrived");
}

if (src == null) {
Expand All @@ -254,7 +254,7 @@ public void onNext(@Nullable final Buffer src) {
subscription.request(1);
}
} catch (Throwable e) {
throw new CodecDecodingException(NettyChannelContentCodec.this, e, "");
throw wrapDecodingException(NettyChannelContentCodec.this, e);
}
}

Expand All @@ -269,7 +269,7 @@ public void onComplete() {
try {
cleanup(channel);
} catch (Throwable t) {
subscriber.onError(new CodecDecodingException(NettyChannelContentCodec.this, t, ""));
subscriber.onError(wrapDecodingException(NettyChannelContentCodec.this, t));
return;
}

Expand Down Expand Up @@ -348,4 +348,12 @@ private static void safeCleanup(final EmbeddedChannel channel) {
LOGGER.error("Error while closing embedded channel", t);
}
}

private static CodecEncodingException wrapEncodingException(final ContentCodec codec, final Throwable cause) {
return new CodecEncodingException(codec, "Unexpected exception during encoding", cause);
}

private static CodecDecodingException wrapDecodingException(final ContentCodec codec, final Throwable cause) {
return new CodecDecodingException(codec, "Unexpected exception during decoding", cause);
}
}

0 comments on commit 1a7e144

Please sign in to comment.