Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicated messages in exception forwards #6504

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Fix duplicated messages in exception forwards #6504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3dd2c5
Fix exceptions accidently caught by sendMessageOrThrow
nielsm5 Mar 22, 2024
12641bc
Deduplicate exception message
nielsm5 Mar 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions cmis/src/main/java/org/frankframework/extensions/cmis/CmisHttpSender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2018-2020 Nationale-Nederlanden, 2020-2022 WeAreFrank!
Copyright 2018-2020 Nationale-Nederlanden, 2020-2024 WeAreFrank!

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -38,9 +38,9 @@
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.entity.BufferedHttpEntity;
import org.apache.http.entity.ByteArrayEntity;

import org.frankframework.core.PipeLineSession;
import org.frankframework.core.SenderException;
import org.frankframework.core.SenderResult;
import org.frankframework.http.HttpResponseHandler;
import org.frankframework.http.HttpSenderBase;
import org.frankframework.parameters.ParameterValueList;
Expand Down Expand Up @@ -141,6 +141,10 @@ public HttpRequestBase getMethod(URI uri, Message message, ParameterValueList pv
return method;
}

protected boolean validateResponseCode(int statusCode) {
return true; //Always success
}

@Override
public Message extractResult(HttpResponseHandler responseHandler, PipeLineSession session) throws IOException {
int responseCode = -1;
Expand All @@ -149,8 +153,6 @@ public Message extractResult(HttpResponseHandler responseHandler, PipeLineSessio
responseCode = statusline.getStatusCode();

Message responseMessage = responseHandler.getResponseMessage();
responseMessage.closeOnCloseOf(session, this);

InputStream responseStream = null;
InputStream errorStream = null;
Map<String, List<String>> headerFields = responseHandler.getHeaderFields();
Expand All @@ -161,30 +163,33 @@ public Message extractResult(HttpResponseHandler responseHandler, PipeLineSessio
errorStream = responseMessage.asInputStream();
}
Response response = new Response(responseCode, statusline.toString(), headerFields, responseStream, errorStream);
session.put("response", response);
session.put("__response", response);
}
catch(Exception e) {
throw new CmisConnectionException(getUrl(), responseCode, e);
}

return new Message("response");
return Message.nullMessage();
}

public Response invoke(HttpMethod method, String url, Map<String, String> headers, Output writer, BindingSession session) {
//Prepare the message. We will overwrite things later...
int responseCode = -1;

PipeLineSession pls = new PipeLineSession();
pls.put("writer", writer);
pls.put("url", url);
pls.put("method", method);
pls.put("headers", headers);

try (Message ignored = sendMessageOrThrow(Message.nullMessage(), pls)) {
// Message is unused, we use 'Output writer' instead
return (Response) pls.get("response");
} catch (Exception e) {
throw new CmisConnectionException(getUrl(), responseCode, e);
try(PipeLineSession pls = new PipeLineSession()) {
pls.put("writer", writer);
pls.put("url", url);
pls.put("method", method);
pls.put("headers", headers);

try {
// Message is unused, we use 'Output writer' instead
SenderResult ignored = sendMessage(Message.nullMessage(), pls);
ignored.getResult().close(); // close our nullMessage
return (Response) pls.get("__response");
} catch (Exception e) {
throw new CmisConnectionException(getUrl(), responseCode, e);
}
}
}

Expand Down
55 changes: 43 additions & 12 deletions cmis/src/test/java/org/frankframework/extensions/cmis/CmisHttpInvokerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package org.frankframework.extensions.cmis;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
Expand All @@ -9,6 +13,7 @@
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URL;
import java.util.HashMap;
Expand Down Expand Up @@ -40,7 +45,7 @@ public void setup() {
when(session.get(eq(SessionParameter.USER_AGENT), anyString())).thenReturn("Mockito mock-agent");
}

private CmisHttpInvoker createInvoker() {
private CmisHttpInvoker createInvoker(int statusCode) {
return new CmisHttpInvoker() {
@Override
protected CmisHttpSender createSender() {
Expand All @@ -50,7 +55,7 @@ protected CmisHttpSender createSender() {
CloseableHttpClient httpClient = mock(CloseableHttpClient.class);

//Mock all requests
when(httpClient.execute(any(HttpHost.class), any(HttpRequestBase.class), any(HttpContext.class))).thenAnswer(new HttpResponseMock());
when(httpClient.execute(any(HttpHost.class), any(HttpRequestBase.class), any(HttpContext.class))).thenAnswer(new HttpResponseMock(statusCode));
when(sender.getHttpClient()).thenReturn(httpClient);

return sender;
Expand All @@ -75,8 +80,10 @@ public void write(OutputStream out) throws Exception {
};
}

private void assertResponse(String string, Response response) throws IOException {
String result = StreamUtil.streamToString(response.getStream());
private void assertResponse(String string, InputStream response) throws IOException {
assertResponse(string, StreamUtil.streamToString(response));
}
private void assertResponse(String string, String result) throws IOException {
String expected = TestFileUtils.getTestFile(string);
assertNotNull("cannot find test file", expected);

Expand All @@ -85,42 +92,66 @@ private void assertResponse(String string, Response response) throws IOException

@Test
public void testGet() throws Exception {
CmisHttpInvoker invoker = createInvoker();
CmisHttpInvoker invoker = createInvoker(200);
UrlBuilder url = new UrlBuilder("https://dummy.url.com");
Response response = invoker.invokeGET(url, session);

assertResponse("/HttpInvokerResponse/simpleGet.txt", response);
assertNull(response.getErrorContent());
assertNotNull(response.getStream());
assertEquals(200, response.getResponseCode());
assertResponse("/HttpInvokerResponse/simpleGet.txt", response.getStream());
}

@Test
public void testPost() throws Exception {
CmisHttpInvoker invoker = createInvoker();
CmisHttpInvoker invoker = createInvoker(200);
UrlBuilder url = new UrlBuilder("https://dummy.url.com");
Output writer = createOutputFromFile("/HttpInvokerRequest/postMessage.txt");
Response response = invoker.invokePOST(url, "text/plain", writer, session);

assertResponse("/HttpInvokerResponse/simplePost.txt", response);
assertNull(response.getErrorContent());
assertNotNull(response.getStream());
assertEquals(200, response.getResponseCode());
assertResponse("/HttpInvokerResponse/simplePost.txt", response.getStream());
}

@Test
public void testPut() throws Exception {
CmisHttpInvoker invoker = createInvoker();
CmisHttpInvoker invoker = createInvoker(200);
UrlBuilder url = new UrlBuilder("https://dummy.url.com");
Output writer = createOutputFromFile("/HttpInvokerRequest/putMessage.txt");
Map<String, String> headers = new HashMap<>();
headers.put("test-header", "test-value");

Response response = invoker.invokePUT(url, "text/plain", headers, writer, session);

assertResponse("/HttpInvokerResponse/simplePut.txt", response);
assertNull(response.getErrorContent());
assertNotNull(response.getStream());
assertEquals(200, response.getResponseCode());
assertResponse("/HttpInvokerResponse/simplePut.txt", response.getStream());
}

@Test
public void testDelete() throws Exception {
CmisHttpInvoker invoker = createInvoker();
CmisHttpInvoker invoker = createInvoker(200);
UrlBuilder url = new UrlBuilder("https://dummy.url.com");
Response response = invoker.invokeDELETE(url, session);

assertResponse("/HttpInvokerResponse/simpleDelete.txt", response);
assertNull(response.getErrorContent());
assertNotNull(response.getStream());
assertEquals(200, response.getResponseCode());
assertResponse("/HttpInvokerResponse/simpleDelete.txt", response.getStream());
}

@Test
public void testException() throws Exception {
CmisHttpInvoker invoker = createInvoker(400);
UrlBuilder url = new UrlBuilder("https://dummy.url.com");
Response response = invoker.invokeGET(url, session);
assertNotNull(response.getErrorContent());
assertNull(response.getStream());
assertEquals(400, response.getResponseCode());
assertTrue(response.getErrorContent().contains("HOST dummy.url.com"));
assertResponse("/HttpInvokerResponse/simpleGet.txt", response.getErrorContent());
}
}
11 changes: 10 additions & 1 deletion core/src/main/java/org/frankframework/processors/ExceptionHandlingPipeProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.time.Instant;
import java.util.Map;

import org.frankframework.core.INamedObject;
import org.frankframework.core.IPipe;
import org.frankframework.core.PipeForward;
import org.frankframework.core.PipeLine;
Expand Down Expand Up @@ -45,8 +46,16 @@ protected PipeRunResult processPipe(PipeLine pipeLine, IPipe pipe, Message messa
tsReceivedLong = tsReceivedDate.toEpochMilli();
}

final Message errorMessage;
ErrorMessageFormatter emf = new ErrorMessageFormatter();
Message errorMessage = emf.format(e.getMessage(), e.getCause(), pipeLine.getOwner(), message, pipeLineSession.getMessageId(), tsReceivedLong);

if(e instanceof PipeRunException) {
INamedObject location = ((PipeRunException) e).getPipeInError();
errorMessage = emf.format(null, e.getCause(), location, message, pipeLineSession.getMessageId(), tsReceivedLong);
} else {
errorMessage = emf.format(null, e, pipeLine.getOwner(), message, pipeLineSession.getMessageId(), tsReceivedLong);
}

log.info("exception occured, forwarding to exception-forward [{}], exception:\n", PipeForward.EXCEPTION_FORWARD_NAME, e);
return new PipeRunResult(pipe.getForwards().get(PipeForward.EXCEPTION_FORWARD_NAME), errorMessage);
}
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/resources/SpringApplicationContext.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,15 @@
scope="prototype"
>
<property name="pipeProcessor">
<!-- allows a managed exception to be stored in the session -->
<bean
class="org.frankframework.processors.ExceptionHandlingPipeProcessor"
class="org.frankframework.processors.InputOutputPipeProcessor"
autowire="byName"
scope="prototype"
>
<property name="pipeProcessor">
<bean
class="org.frankframework.processors.InputOutputPipeProcessor"
class="org.frankframework.processors.ExceptionHandlingPipeProcessor"
autowire="byName"
scope="prototype"
>
Expand Down
11 changes: 10 additions & 1 deletion core/src/test/java/org/frankframework/http/HttpResponseMock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,23 @@
import lombok.Getter;

public class HttpResponseMock extends Mockito implements Answer<HttpResponse> {

private final String lineSeparator = System.getProperty("line.separator");
private final int statusCode;

public HttpResponseMock() {
this(200);
}
public HttpResponseMock(int statusCode) {
this.statusCode = statusCode;
}

private HttpResponse buildResponse(InputStream content) throws UnsupportedOperationException, IOException {
CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class);
StatusLine statusLine = mock(StatusLine.class);
HttpEntity httpEntity = mock(HttpEntity.class);

when(statusLine.getStatusCode()).thenReturn(200);
when(statusLine.getStatusCode()).thenReturn(statusCode);
when(httpResponse.getStatusLine()).thenReturn(statusLine);

when(httpEntity.getContent()).thenReturn(content);
Expand Down
6 changes: 3 additions & 3 deletions test/src/test/testtool/ClobAndBlob/scenario07/out.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<errorMessage timestamp="Thu Feb 24 12:10:59 CET 2022" originator="IAF_ss9 7.8-SNAPSHOT" message="Adapter [ClobAndBlob] msgId [synthetic-message-id-a9fee1b8-45cd1ffc_18c35bfd289_-7f68]: Pipe [readBlobReadAsIfcompressed] caught exception: [org.frankframework.jdbc.FixedQuerySender] [readBlobReadAsIfcompressed-sender] got exception executing a SELECT SQL command: (ZipException) incorrect header check: [org.frankframework.jdbc.FixedQuerySender] [readBlobReadAsIfcompressed-sender] got exception executing a SELECT SQL command: (ZipException) incorrect header check">
<location class="org.frankframework.core.Adapter" name="ClobAndBlob"/>
<errorMessage timestamp="IGNORE" originator="IAFIGNORE" message="SenderPipe [readBlobReadAsIfcompressed] msgId [IGNORE]: [org.frankframework.jdbc.FixedQuerySender] [readBlobReadAsIfcompressed-sender] got exception executing a SELECT SQL command: (ZipException) incorrect header check">
<location class="org.frankframework.pipes.SenderPipe" name="readBlobReadAsIfcompressed"/>
<details>IGNORE</details>
<originalMessage messageId="IGNORE" receivedTime="IGNORE">&lt;result/&gt;</originalMessage>
</errorMessage>
</errorMessage>
10 changes: 5 additions & 5 deletions test/src/test/testtool/Senders/scenario01/out.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<errorMessage timestamp="Mon Dec 04 18:20:24 CET 2023" originator="IAF_ss9 8.0-SNAPSHOT" message="Adapter [SendersExceptionInSenderWithExceptionForward] msgId [synthetic-message-id-a9fee1b8--28ad0d7c_18c35d5dec4_-7f68]: Pipe [Call ExceptionPipe] caught exception: IbisLocalSender [Call ExceptionPipe-sender] exception calling JavaListener [ExceptionPipe]: Pipe [Generate exception] &lt;test/&gt;: IbisLocalSender [Call ExceptionPipe-sender] exception calling JavaListener [ExceptionPipe]: Pipe [Generate exception] &lt;test/&gt;">
<location class="org.frankframework.core.Adapter" name="SendersExceptionInSenderWithExceptionForward"/>
<details>IGNORE</details>
<originalMessage messageId="synthetic-message-id-a9fee1b8--28ad0d7c_18c35d5dec4_-7f68" receivedTime="Mon Dec 04 18:20:23 CET 2023">&lt;test/&gt;</originalMessage>
</errorMessage>
<errorMessage timestamp="IGNORE" originator="IAFIGNORE" message="SenderPipe [Call ExceptionPipe] msgId [IGNORE]: IbisLocalSender [Call ExceptionPipe-sender] exception calling JavaListener [ExceptionPipe]: Pipe [Generate exception] &lt;test/&gt;">
<location class="org.frankframework.pipes.SenderPipe" name="Call ExceptionPipe"/>
<details>IGNORE</details>
<originalMessage messageId="IGNORE" receivedTime="IGNORE">&lt;test/&gt;</originalMessage>
</errorMessage>
2 changes: 1 addition & 1 deletion test/src/test/testtool/UnzipPipeExceptionForwardTest/scenario01/out1.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Adapter [UnzipPipe] msgId [0a722fe4--2c744fc_183eb0a3993_-7aa3]: Pipe [unzipFile] cannot unzip: (EOFException) Unexpected end of ZLIB input stream: Unexpected end of ZLIB input stream
UnzipPipe [unzipFile] msgId [synthetic-message-id-c0a80198--4d3d00bd_18e6bcf8d17_-7bfc]: Unexpected end of ZLIB input stream
Loading