Skip to content

Commit 37cbb39

Browse files
CesarCoelhoclaude
andcommitted
Return errors from the endpoint the message was addressed to
returnErrorMessage picked an arbitrary endpoint to build the reply from, so the error carried the URI, supplements and transport specific header of whichever endpoint the map happened to iterate first. dispatchMessage did have the right endpoint, but declared it inside the try block where the catch clauses could not see it. Hoist it, and pass it down, so an error caused by a failing delivery is returned from the endpoint the message was actually addressed to. The endpoint-not-found path still has no endpoint to attribute to and keeps falling back to any of them. Move the construction and sending of the reply into a new ErrorReplyBuilder while here. The existing three argument returnErrorMessage keeps its signature and delegates, so the two callers outside this class, both decode failures with no endpoint context, are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 7fad9da commit 37cbb39

2 files changed

Lines changed: 177 additions & 54 deletions

File tree

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/* ----------------------------------------------------------------------------
2+
* Copyright (C) 2013 European Space Agency
3+
* European Space Operations Centre
4+
* Darmstadt
5+
* Germany
6+
* ----------------------------------------------------------------------------
7+
* System : CCSDS MO Generic Transport Framework
8+
* ----------------------------------------------------------------------------
9+
* Licensed under the European Space Agency Public License, Version 2.0
10+
* You may not use this file except in compliance with the License.
11+
*
12+
* Except as expressly set forth in this License, the Software is provided to
13+
* You on an "as is" basis and without warranties of any kind, including without
14+
* limitation merchantability, fitness for a particular purpose, absence of
15+
* defects or errors, accuracy or non-infringement of intellectual property rights.
16+
*
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
* ----------------------------------------------------------------------------
20+
*/
21+
package esa.mo.mal.transport.gen;
22+
23+
import java.util.Map;
24+
import java.util.logging.Level;
25+
import org.ccsds.moims.mo.mal.MALException;
26+
import org.ccsds.moims.mo.mal.MALInvokeOperation;
27+
import org.ccsds.moims.mo.mal.MALProgressOperation;
28+
import org.ccsds.moims.mo.mal.MALPubSubOperation;
29+
import org.ccsds.moims.mo.mal.MALRequestOperation;
30+
import org.ccsds.moims.mo.mal.MALSubmitOperation;
31+
import org.ccsds.moims.mo.mal.structures.InteractionType;
32+
import org.ccsds.moims.mo.mal.structures.Time;
33+
import org.ccsds.moims.mo.mal.structures.UInteger;
34+
import org.ccsds.moims.mo.mal.structures.UOctet;
35+
import org.ccsds.moims.mo.mal.structures.Union;
36+
import org.ccsds.moims.mo.mal.transport.MALMessageHeader;
37+
import org.ccsds.moims.mo.mal.transport.MALTransmitErrorException;
38+
39+
/**
40+
* Builds and sends the MO Error message that answers a received message which
41+
* could not be delivered or processed.
42+
*
43+
* Only certain interaction type and stage combinations have an error defined as
44+
* their answer; for all others no error is returned.
45+
*/
46+
public class ErrorReplyBuilder {
47+
48+
private final Transport transport;
49+
private final EndpointRegistry endpoints;
50+
private final Map qosProperties;
51+
52+
/**
53+
* Constructor.
54+
*
55+
* @param transport The transport used to send the error message.
56+
* @param endpoints The endpoints of that transport, used to find a fallback
57+
* sender when the intended endpoint is not known.
58+
* @param qosProperties The QoS properties to send the error message with.
59+
*/
60+
public ErrorReplyBuilder(final Transport transport,
61+
final EndpointRegistry endpoints, final Map qosProperties) {
62+
this.transport = transport;
63+
this.endpoints = endpoints;
64+
this.qosProperties = qosProperties;
65+
}
66+
67+
/**
68+
* Returns true if a message with this interaction type and stage has an MO
69+
* Error defined as a valid answer.
70+
*
71+
* @param interactionType The interaction type of the received message.
72+
* @param stage The interaction stage of the received message.
73+
* @return True if an error may be returned.
74+
*/
75+
private static boolean isErrorExpected(final InteractionType interactionType, final short stage) {
76+
return ((interactionType.equals(InteractionType.SUBMIT)) && (stage == MALSubmitOperation._SUBMIT_STAGE))
77+
|| ((interactionType.equals(InteractionType.REQUEST)) && (stage == MALRequestOperation._REQUEST_STAGE))
78+
|| ((interactionType.equals(InteractionType.INVOKE)) && (stage == MALInvokeOperation._INVOKE_STAGE))
79+
|| ((interactionType.equals(InteractionType.PROGRESS)) && (stage == MALProgressOperation._PROGRESS_STAGE))
80+
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._REGISTER_STAGE))
81+
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._DEREGISTER_STAGE))
82+
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._PUBLISH_REGISTER_STAGE))
83+
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._PUBLISH_DEREGISTER_STAGE));
84+
}
85+
86+
/**
87+
* Creates and sends an error message answering a received message.
88+
*
89+
* The error is sent from the endpoint the received message was being
90+
* delivered to. When that endpoint is not known, for example because the
91+
* message could not be routed to one at all, any endpoint of the transport
92+
* is used instead so that the sender is at least informed.
93+
*
94+
* @param endpoint The endpoint the message was being delivered to, or null
95+
* if it is not known.
96+
* @param srcHdr The header of the received message.
97+
* @param errorNumber The error number.
98+
* @param errorMsg The error message.
99+
* @throws MALException if the response message could not be encoded.
100+
*/
101+
public void returnError(final Endpoint endpoint, final MALMessageHeader srcHdr,
102+
final UInteger errorNumber, final String errorMsg) throws MALException {
103+
try {
104+
InteractionType interactionType = srcHdr.getInteractionType();
105+
final short stage = (null != srcHdr.getInteractionStage())
106+
? srcHdr.getInteractionStage().getValue() : 0;
107+
108+
if (!isErrorExpected(interactionType, stage)) {
109+
Transport.LOGGER.log(Level.WARNING, "An MO Error will not be returned because this "
110+
+ "combination of type/stage does not have an MO Error to "
111+
+ "be returned! For interaction type: {0} - and stage: {1}",
112+
new Object[]{interactionType.toString(), stage});
113+
return;
114+
}
115+
116+
Endpoint sender = (endpoint != null) ? endpoint : endpoints.any();
117+
118+
if (sender == null) {
119+
Transport.LOGGER.log(Level.WARNING, "(1) Unable to return error"
120+
+ " number ({0}) as no endpoint supplied: {1}",
121+
new Object[]{errorNumber, srcHdr});
122+
return;
123+
}
124+
125+
final GENMessage retMsg = (GENMessage) sender.createMessage(srcHdr.getAuthenticationId(),
126+
srcHdr.getFromURI(),
127+
Time.now(),
128+
srcHdr.getInteractionType(),
129+
new UOctet((short) (srcHdr.getInteractionStage().getValue() + 1)),
130+
srcHdr.getTransactionId(),
131+
srcHdr.getServiceArea(),
132+
srcHdr.getService(),
133+
srcHdr.getOperation(),
134+
srcHdr.getAreaVersion(),
135+
true,
136+
srcHdr.getSupplements(),
137+
qosProperties,
138+
errorNumber, new Union(errorMsg));
139+
140+
transport.sendMessage(null, true, retMsg);
141+
} catch (MALTransmitErrorException ex) {
142+
Transport.LOGGER.log(Level.WARNING,
143+
"Error occurred when attempting to return previous error!",
144+
ex);
145+
}
146+
}
147+
}

transports/transport-generic/src/main/java/esa/mo/mal/transport/gen/Transport.java

Lines changed: 30 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.ccsds.moims.mo.mal.*;
3838
import org.ccsds.moims.mo.mal.encoding.MALElementStreamFactory;
3939
import org.ccsds.moims.mo.mal.structures.*;
40-
import org.ccsds.moims.mo.mal.structures.InteractionType;
4140
import org.ccsds.moims.mo.mal.transport.*;
4241

4342
/**
@@ -95,6 +94,11 @@ public abstract class Transport<I, O> implements MALTransport {
9594
* transport routing name.
9695
*/
9796
protected final EndpointRegistry endpoints = new EndpointRegistry();
97+
/**
98+
* Builds and sends the error messages that answer messages which could not
99+
* be delivered or processed.
100+
*/
101+
protected final ErrorReplyBuilder errorReplies;
98102
/**
99103
* Map of QoS properties.
100104
*/
@@ -164,6 +168,7 @@ public Transport(final String protocol,
164168
this.addressing = new TransportAddressing(protocol, protocolDelim,
165169
serviceDelim, routingDelim, supportsRouting);
166170
this.qosProperties = properties;
171+
this.errorReplies = new ErrorReplyBuilder(this, endpoints, properties);
167172

168173
streamFactory = MALElementStreamFactory.newFactory(protocol, properties);
169174
LOGGER.log(Level.FINE, "Created element stream: {0}",
@@ -440,12 +445,16 @@ public void receiveIncomingMessage(final IncomingMessageHolder malMsg) {
440445
* @param smsg The message in a string representation for logging.
441446
*/
442447
public void dispatchMessage(final GENMessage msg, PacketToString smsg) {
448+
// Held outside the try so that, if the delivery below throws, the error
449+
// can be returned from the endpoint the message was destined for.
450+
Endpoint endpoint = null;
451+
443452
try {
444453
LOGGER.log(Level.FINE, "Processing message : {0} : {1}",
445454
new Object[]{msg.getHeader().getTransactionId(), smsg});
446455

447456
String endpointUriPart = getRoutingPart(msg.getHeader().getTo().getValue());
448-
final Endpoint endpoint = endpoints.getByRoutingName(endpointUriPart);
457+
endpoint = endpoints.getByRoutingName(endpointUriPart);
449458

450459
if (endpoint != null) {
451460
LOGGER.log(Level.FINE, "Passing message to endpoint {0} : {1}",
@@ -466,7 +475,7 @@ public void dispatchMessage(final GENMessage msg, PacketToString smsg) {
466475
e.printStackTrace(new PrintWriter(wrt));
467476

468477
try {
469-
returnErrorMessage(msg.getHeader(), MALHelper.INTERNAL_ERROR_NUMBER,
478+
returnErrorMessage(endpoint, msg.getHeader(), MALHelper.INTERNAL_ERROR_NUMBER,
470479
"Error occurred: " + e.toString() + " : " + wrt.toString());
471480
} catch (MALException ex) {
472481
LOGGER.log(Level.SEVERE,
@@ -482,7 +491,7 @@ public void dispatchMessage(final GENMessage msg, PacketToString smsg) {
482491
e.printStackTrace(new PrintWriter(wrt));
483492

484493
try {
485-
returnErrorMessage(msg.getHeader(), MALHelper.INTERNAL_ERROR_NUMBER,
494+
returnErrorMessage(endpoint, msg.getHeader(), MALHelper.INTERNAL_ERROR_NUMBER,
486495
"Error occurred: " + e.toString() + " : " + wrt.toString());
487496
} catch (MALException ex) {
488497
LOGGER.log(Level.SEVERE, "Error occurred when return error data : {0}", ex);
@@ -500,56 +509,23 @@ public void dispatchMessage(final GENMessage msg, PacketToString smsg) {
500509
*/
501510
protected void returnErrorMessage(final MALMessageHeader srcHdr,
502511
final UInteger errorNumber, final String errorMsg) throws MALException {
503-
try {
504-
InteractionType interactionType = srcHdr.getInteractionType();
505-
final short stage = (null != srcHdr.getInteractionStage())
506-
? srcHdr.getInteractionStage().getValue() : 0;
507-
508-
// first check that message should be responded to
509-
if (((interactionType.equals(InteractionType.SUBMIT)) && (stage == MALSubmitOperation._SUBMIT_STAGE))
510-
|| ((interactionType.equals(InteractionType.REQUEST)) && (stage == MALRequestOperation._REQUEST_STAGE))
511-
|| ((interactionType.equals(InteractionType.INVOKE)) && (stage == MALInvokeOperation._INVOKE_STAGE))
512-
|| ((interactionType.equals(InteractionType.PROGRESS)) && (stage == MALProgressOperation._PROGRESS_STAGE))
513-
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._REGISTER_STAGE))
514-
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._DEREGISTER_STAGE))
515-
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._PUBLISH_REGISTER_STAGE))
516-
|| ((interactionType.equals(InteractionType.PUBSUB)) && (stage == MALPubSubOperation._PUBLISH_DEREGISTER_STAGE))) {
517-
518-
Endpoint endpoint = endpoints.any();
519-
520-
if (endpoint != null) {
521-
final GENMessage retMsg = (GENMessage) endpoint.createMessage(srcHdr.getAuthenticationId(),
522-
srcHdr.getFromURI(),
523-
Time.now(),
524-
srcHdr.getInteractionType(),
525-
new UOctet((short) (srcHdr.getInteractionStage().getValue() + 1)),
526-
srcHdr.getTransactionId(),
527-
srcHdr.getServiceArea(),
528-
srcHdr.getService(),
529-
srcHdr.getOperation(),
530-
srcHdr.getAreaVersion(),
531-
true,
532-
srcHdr.getSupplements(),
533-
qosProperties,
534-
errorNumber, new Union(errorMsg));
535-
536-
sendMessage(null, true, retMsg);
537-
} else {
538-
LOGGER.log(Level.WARNING, "(1) Unable to return error"
539-
+ " number ({0}) as no endpoint supplied: {1}",
540-
new Object[]{errorNumber, srcHdr});
541-
}
542-
} else {
543-
LOGGER.log(Level.WARNING, "An MO Error will not be returned because this "
544-
+ "combination of type/stage does not have an MO Error to "
545-
+ "be returned! For interaction type: {0} - and stage: {1}",
546-
new Object[]{interactionType.toString(), stage});
547-
}
548-
} catch (MALTransmitErrorException ex) {
549-
LOGGER.log(Level.WARNING,
550-
"Error occurred when attempting to return previous error!",
551-
ex);
552-
}
512+
returnErrorMessage(null, srcHdr, errorNumber, errorMsg);
513+
}
514+
515+
/**
516+
* Creates a return error message based on a received message, sent from the
517+
* endpoint the received message was being delivered to.
518+
*
519+
* @param endpoint The endpoint the message was being delivered to, or null
520+
* if it is not known.
521+
* @param srcHdr The source header
522+
* @param errorNumber The error number
523+
* @param errorMsg The error message.
524+
* @throws MALException if cannot encode a response message
525+
*/
526+
protected void returnErrorMessage(final Endpoint endpoint, final MALMessageHeader srcHdr,
527+
final UInteger errorNumber, final String errorMsg) throws MALException {
528+
errorReplies.returnError(endpoint, srcHdr, errorNumber, errorMsg);
553529
}
554530

555531
/**

0 commit comments

Comments
 (0)