sergey-chugunov-1985 commented on code in PR #12555:
URL: https://github.com/apache/ignite/pull/12555#discussion_r2645482922
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceDeploymentTask.java:
##########
@@ -541,6 +541,15 @@ private void completeInitiatingFuture(final Throwable err)
{
}
catch (IgniteCheckedException e) {
log.error("Failed to unmarshal deployment error.", e);
+
+ Exception ex = new IgniteCheckedException(
+ "Failed to unmarshal deployment error! See server logs
for details."
Review Comment:
```suggestion
"Failed to unmarshal deployment error, see server
logs for details."
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceDeploymentTask.java:
##########
@@ -685,6 +694,19 @@ private void attachDeploymentErrors(@NotNull
ServiceSingleNodeDeploymentResult d
}
catch (IgniteCheckedException e) {
log.error("Failed to marshal deployment error, err=" + th, e);
+
+ try {
+ Exception ex = new IgniteCheckedException(
+ "Failed to marshal deployment error! See server logs
for details, err=" + th
Review Comment:
```suggestion
"Failed to marshal deployment error, see server logs
for details. Deployment error: " + th
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceDeploymentTask.java:
##########
@@ -685,6 +694,19 @@ private void attachDeploymentErrors(@NotNull
ServiceSingleNodeDeploymentResult d
}
catch (IgniteCheckedException e) {
log.error("Failed to marshal deployment error, err=" + th, e);
+
+ try {
+ Exception ex = new IgniteCheckedException(
+ "Failed to marshal deployment error! See server logs
for details, err=" + th
+ );
+
+ byte[] arr = U.marshal(ctx, ex);
+
+ errorsBytes.add(arr);
+ }
+ catch (IgniteCheckedException ex) {
+ log.error("Failed to attach marshal deployment error to
the message, err=" + th, ex);
Review Comment:
```suggestion
log.error("Failed to attach deployment error information
to deployment result message", ex);
```
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceExceptionPropagationTest.java:
##########
@@ -0,0 +1,542 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.service;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.io.Serializable;
+import java.lang.reflect.Field;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.services.ServiceDeploymentException;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** */
+public class GridServiceExceptionPropagationTest extends
GridCommonAbstractTest {
+ /** */
+ private static final String EX_BROKEN_SER_MSG = "Exception occurred on
serialization step";
+
+ /** */
+ private static final String RETURNED_EX_BROKEN_SER_MSG = "See server logs
for details";
+
+ /** */
+ private static final String EX_MSG = "Exception message";
+
+ /** */
+ private static final String SERVICE_NAME = "my-service";
+
+ /** */
+ private static final ExceptionThrower SERIAL_EX =
ExceptionThrower.serializable();
+
+ /** */
+ private static final ExceptionThrower EXT_EX =
ExceptionThrower.externalizable(false, false);
+
+ /** */
+ private static final ExceptionThrower EX_WITH_BROKEN_SER =
ExceptionThrower.externalizable(true, true);
+
+ /** */
+ private static final ExceptionThrower EX_WITH_BROKEN_DESER =
ExceptionThrower.externalizable(true, false);
+
+ /** */
+ private boolean isNodeInfoAvailableInExMsg = true;
+
+ /** */
+ @Test
+ public void testServiceCancelThrowsSerializableException() throws
Exception {
+ Service svc = new
ServiceWithException().withCancelException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void testServiceCancelThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withCancelException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void
testServiceCancelThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withCancelException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void
testServiceCancelThrowsExternalizableExceptionWithBrokenDeserialization()
throws Exception {
+ Service svc = new
ServiceWithException().withCancelException(EX_WITH_BROKEN_DESER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void testServiceInitThrowsSerializableException() throws Exception {
+ Service svc = new ServiceWithException().withInitException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void testServiceInitThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withInitException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceInitThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withInitException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceInitThrowsExternalizableExceptionWithBrokenDeserialization() throws
Exception {
+ Service svc = new
ServiceWithException().withInitException(EX_WITH_BROKEN_DESER);
+
+ isNodeInfoAvailableInExMsg = false;
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+
+ isNodeInfoAvailableInExMsg = true;
+ }
+
+ /** */
+ @Test
+ public void testServiceExecuteThrowsSerializableException() throws
Exception {
+ Service svc = new
ServiceWithException().withExecuteException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void testServiceExecuteThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withExecuteException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceExecuteThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withExecuteException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceExecuteThrowsExternalizableExceptionWithBrokenDeserialization()
throws Exception {
+ Service svc = new
ServiceWithException().withExecuteException(EX_WITH_BROKEN_DESER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ private void testExceptionPropagation(
+ ServiceConfiguration srvcCfg,
+ boolean shouldThrow,
+ boolean withCancel
+ ) throws Exception {
+ try (IgniteEx srv = startGrid(0); IgniteEx client =
startClientGrid(1)) {
+ try {
+ client.services().deploy(srvcCfg);
+
+ if (withCancel)
+ client.services().cancel(SERVICE_NAME);
+
+ if (shouldThrow)
+ fail("Exception has been expected.");
+ }
+ catch (ServiceDeploymentException ex) {
+ assertTrue(shouldThrow);
+
+ String errMsg = ex.getSuppressed()[0].getMessage();
+
+ if (isNodeInfoAvailableInExMsg) {
+
assertTrue(errMsg.contains(srv.cluster().localNode().id().toString()));
+ assertTrue(errMsg.contains(SERVICE_NAME));
+ }
+
+ Throwable cause = ex.getSuppressed()[0].getCause();
+
+ if (cause == null)
+ assertTrue(errMsg.contains(RETURNED_EX_BROKEN_SER_MSG));
+ else
+ assertTrue(cause.getMessage().contains(EX_MSG));
+ }
+ catch (Throwable e) {
+ throw new AssertionError("Unexpected exception has been
thrown.", e);
+ }
+ }
+ }
+
+ /** */
+ private ServiceConfiguration getServiceConfiguration(Service svc) {
+ return new ServiceConfiguration()
+ .setService(svc)
+ .setName(SERVICE_NAME)
+ .setMaxPerNodeCount(1)
+ .setNodeFilter(new ClientNodeFilter());
+ }
+
+ /**
+ *
+ */
+ private static class ClientNodeFilter implements
IgnitePredicate<ClusterNode> {
+ /** {@inheritDoc} */
+ @Override public boolean apply(ClusterNode node) {
+ return !node.isClient();
+ }
+ }
+
+ /**
+ * A simple {@link Service} implementation that intentionally throws
exceptions during
+ * specific {@link Service} lifecycle phases for testing purposes.
+ *
+ * <p>This service can be configured to:
+ * <ul>
+ * <li>Throw an {@link Exception} during either {@link #init()} or
{@link #execute()}.</li>
+ * <li>Throw a {@link RuntimeException} during {@link #cancel()}.</li>
+ * </ul>
+ */
+ private static class ServiceWithException implements Service {
+ /** */
+ private boolean throwOnCancel;
+
+ /** */
+ private boolean throwOnInit;
+
+ /** */
+ private boolean throwOnExec;
+
+ /** */
+ private ExceptionThrower exThrower;
+
+ /** {@inheritDoc} */
+ @Override public void cancel() {
+ if (throwOnCancel)
+ exThrower.throwRunnableException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public void init() throws Exception {
+ if (throwOnInit)
+ exThrower.throwException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public void execute() throws Exception {
+ if (throwOnExec)
+ exThrower.throwException();
+ }
+
+ /** */
+ public Service withCancelException(ExceptionThrower exThrower) {
+ this.exThrower = exThrower;
+
+ throwOnCancel = true;
+
+ return this;
+ }
+
+ /** */
+ public Service withInitException(ExceptionThrower exThrower) {
+ this.exThrower = exThrower;
+
+ throwOnInit = true;
+
+ return this;
+ }
+
+ /** */
+ public Service withExecuteException(ExceptionThrower exThrower) {
+ this.exThrower = exThrower;
+
+ throwOnExec = true;
+
+ return this;
+ }
+ }
+
+ /** */
+ private static class ExceptionThrower implements Serializable {
+ /** */
+ private final boolean isSerializable;
+
+ /** */
+ private final boolean isBroken;
+
+ /** */
+ private final boolean isSerializationBroken;
+
+ /** */
+ private ExceptionThrower(boolean isSerializable, boolean isBroken,
boolean isSerializationBroken) {
+ this.isSerializable = isSerializable;
+ this.isBroken = isBroken;
+ this.isSerializationBroken = isSerializationBroken;
+ }
+
+ /**
+ * @return Serializable exception configuration
+ */
+ static ExceptionThrower serializable() {
+ return new ExceptionThrower(true, false, false);
+ }
+
+ /**
+ * @return Externalizable exception configuration
+ */
+ static ExceptionThrower externalizable(boolean isBroken, boolean
isSerializationBroken) {
+ return new ExceptionThrower(false, isBroken,
isSerializationBroken);
+ }
+
+ /** */
+ public void throwException() throws Exception {
+ if (isSerializable)
+ throw new Exception(EX_MSG);
+
+ throw new ExternalizableException(EX_MSG, isBroken,
isSerializationBroken);
+ }
+
+ /** */
+ public void throwRunnableException() {
Review Comment:
```suggestion
public void throwRuntimeException() {
```
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceExceptionPropagationTest.java:
##########
@@ -0,0 +1,542 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.service;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.io.Serializable;
+import java.lang.reflect.Field;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.lang.IgnitePredicate;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.services.ServiceDeploymentException;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/** */
+public class GridServiceExceptionPropagationTest extends
GridCommonAbstractTest {
+ /** */
+ private static final String EX_BROKEN_SER_MSG = "Exception occurred on
serialization step";
+
+ /** */
+ private static final String RETURNED_EX_BROKEN_SER_MSG = "See server logs
for details";
+
+ /** */
+ private static final String EX_MSG = "Exception message";
+
+ /** */
+ private static final String SERVICE_NAME = "my-service";
+
+ /** */
+ private static final ExceptionThrower SERIAL_EX =
ExceptionThrower.serializable();
+
+ /** */
+ private static final ExceptionThrower EXT_EX =
ExceptionThrower.externalizable(false, false);
+
+ /** */
+ private static final ExceptionThrower EX_WITH_BROKEN_SER =
ExceptionThrower.externalizable(true, true);
+
+ /** */
+ private static final ExceptionThrower EX_WITH_BROKEN_DESER =
ExceptionThrower.externalizable(true, false);
+
+ /** */
+ private boolean isNodeInfoAvailableInExMsg = true;
+
+ /** */
+ @Test
+ public void testServiceCancelThrowsSerializableException() throws
Exception {
+ Service svc = new
ServiceWithException().withCancelException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void testServiceCancelThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withCancelException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void
testServiceCancelThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withCancelException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void
testServiceCancelThrowsExternalizableExceptionWithBrokenDeserialization()
throws Exception {
+ Service svc = new
ServiceWithException().withCancelException(EX_WITH_BROKEN_DESER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, true);
+ }
+
+ /** */
+ @Test
+ public void testServiceInitThrowsSerializableException() throws Exception {
+ Service svc = new ServiceWithException().withInitException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void testServiceInitThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withInitException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceInitThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withInitException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceInitThrowsExternalizableExceptionWithBrokenDeserialization() throws
Exception {
+ Service svc = new
ServiceWithException().withInitException(EX_WITH_BROKEN_DESER);
+
+ isNodeInfoAvailableInExMsg = false;
+
+ testExceptionPropagation(getServiceConfiguration(svc), true, false);
+
+ isNodeInfoAvailableInExMsg = true;
+ }
+
+ /** */
+ @Test
+ public void testServiceExecuteThrowsSerializableException() throws
Exception {
+ Service svc = new
ServiceWithException().withExecuteException(SERIAL_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void testServiceExecuteThrowsExternalizableException() throws
Exception {
+ Service svc = new ServiceWithException().withExecuteException(EXT_EX);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceExecuteThrowsExternalizableExceptionWithBrokenSerialization() throws
Exception {
+ Service svc = new
ServiceWithException().withExecuteException(EX_WITH_BROKEN_SER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ @Test
+ public void
testServiceExecuteThrowsExternalizableExceptionWithBrokenDeserialization()
throws Exception {
+ Service svc = new
ServiceWithException().withExecuteException(EX_WITH_BROKEN_DESER);
+
+ testExceptionPropagation(getServiceConfiguration(svc), false, false);
+ }
+
+ /** */
+ private void testExceptionPropagation(
+ ServiceConfiguration srvcCfg,
+ boolean shouldThrow,
+ boolean withCancel
+ ) throws Exception {
+ try (IgniteEx srv = startGrid(0); IgniteEx client =
startClientGrid(1)) {
+ try {
+ client.services().deploy(srvcCfg);
+
+ if (withCancel)
+ client.services().cancel(SERVICE_NAME);
+
+ if (shouldThrow)
+ fail("Exception has been expected.");
Review Comment:
```suggestion
fail("An expected exception has not been thrown.");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
