Skip to content

Commit eef6a26

Browse files
committed
Improve how ImageBuilder.build() handled HTTP 400 ("Bad Request")
1 parent 9a65303 commit eef6a26

File tree

5 files changed

+31
-81
lines changed

5 files changed

+31
-81
lines changed

src/main/java/com/github/cowwoc/docker/internal/util/AsyncResponseListener.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,16 @@ protected AsyncResponseListener(InternalClient client, BlockingQueue<Throwable>
5252
}
5353

5454
/**
55-
* Processes a single JSON object returned by the server.
55+
* Processes a single non-empty line returned by the server.
5656
*
57-
* @param objectAsString the String representation of the JSON object
58-
* @throws NullPointerException if {@code objectAsString} is null
57+
* @param line the line
58+
* @throws NullPointerException if {@code line} is null
5959
*/
60-
protected void processObject(String objectAsString)
60+
protected void processObject(String line)
6161
{
6262
try
6363
{
64-
JsonNode json = client.getJsonMapper().readTree(objectAsString);
64+
JsonNode json = client.getJsonMapper().readTree(line);
6565
JsonNode node = json.get("message");
6666
if (node != null)
6767
{
@@ -76,7 +76,6 @@ protected void processObject(String objectAsString)
7676
warnOnUnexpectedProperties(json, "errorDetail", "error");
7777
warnOnUnexpectedProperties(node, "code", "message");
7878
String message = node.get("message").textValue();
79-
log.error(message);
8079
exceptions.add(new IOException(message));
8180
return;
8281
}
@@ -85,7 +84,6 @@ protected void processObject(String objectAsString)
8584
{
8685
warnOnUnexpectedProperties(json, "error");
8786
String message = node.textValue();
88-
log.error(message);
8987
exceptions.add(new IOException(message));
9088
}
9189
processUnknownProperties(json);

src/main/java/com/github/cowwoc/docker/internal/util/ImageBuildListener.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package com.github.cowwoc.docker.internal.util;
22

33
import com.fasterxml.jackson.databind.JsonNode;
4+
import com.github.cowwoc.docker.exception.ResourceNotFoundException;
45
import com.github.cowwoc.docker.internal.client.InternalClient;
56
import org.eclipse.jetty.client.Response;
67
import org.eclipse.jetty.client.Result;
78

89
import java.io.IOException;
910
import java.util.concurrent.LinkedBlockingQueue;
1011

12+
import static org.eclipse.jetty.http.HttpStatus.BAD_REQUEST_400;
1113
import static org.eclipse.jetty.http.HttpStatus.INTERNAL_SERVER_ERROR_500;
1214
import static org.eclipse.jetty.http.HttpStatus.OK_200;
1315
import static org.eclipse.jetty.util.BufferUtil.EMPTY_BUFFER;
@@ -38,8 +40,7 @@ protected void processUnknownProperties(JsonNode json)
3840
if (node != null)
3941
{
4042
warnOnUnexpectedProperties(json, "stream");
41-
linesToLog.append(node.textValue());
42-
Strings.logLines(linesToLog, log);
43+
log.info(node.textValue());
4344
return;
4445
}
4546
node = json.get("aux");
@@ -72,14 +73,17 @@ public void onComplete(Result result)
7273
exceptions.add(result.getResponseFailure());
7374
decodeBytes(EMPTY_BUFFER, true);
7475
processResponse(true);
75-
if (!linesToLog.isEmpty())
76-
log.info(linesToLog.toString());
7776

7877
// https://docs.docker.com/reference/api/engine/version/v1.47/#tag/Image/operation/ImageBuild
7978
Response response = result.getResponse();
8079
switch (response.getStatus())
8180
{
8281
case OK_200 -> processResponse(true);
82+
case BAD_REQUEST_400 ->
83+
{
84+
JsonNode body = getResponseBody();
85+
exceptions.add(new ResourceNotFoundException(body.get("message").textValue()));
86+
}
8387
case INTERNAL_SERVER_ERROR_500 -> exceptions.add(
8488
new IOException("Unexpected response: " + responseAsString + "\n" +
8589
"Request: " + client.toString(result.getRequest())));

src/main/java/com/github/cowwoc/docker/internal/util/ImageTransferListener.java

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package com.github.cowwoc.docker.internal.util;
22

3-
import com.fasterxml.jackson.core.JsonProcessingException;
43
import com.fasterxml.jackson.databind.JsonNode;
54
import com.github.cowwoc.docker.exception.ResourceNotFoundException;
65
import com.github.cowwoc.docker.internal.client.InternalClient;
7-
import com.github.cowwoc.pouch.core.WrappedCheckedException;
86
import org.eclipse.jetty.client.Response;
97
import org.eclipse.jetty.client.Result;
108

@@ -80,19 +78,4 @@ public void onComplete(Result result)
8078
responseReady.countDown();
8179
}
8280
}
83-
84-
/**
85-
* @return the server response as JSON
86-
*/
87-
private JsonNode getResponseBody()
88-
{
89-
try
90-
{
91-
return client.getJsonMapper().readTree(responseAsString.toString());
92-
}
93-
catch (JsonProcessingException e)
94-
{
95-
throw WrappedCheckedException.wrap(e);
96-
}
97-
}
9881
}

src/main/java/com/github/cowwoc/docker/internal/util/JsonStreamListener.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.github.cowwoc.docker.internal.util;
22

3+
import com.fasterxml.jackson.core.JsonProcessingException;
34
import com.fasterxml.jackson.databind.JsonNode;
45
import com.github.cowwoc.docker.internal.client.InternalClient;
6+
import com.github.cowwoc.pouch.core.WrappedCheckedException;
57
import org.eclipse.jetty.client.Response;
68

79
import java.nio.ByteBuffer;
@@ -38,10 +40,6 @@ public abstract class JsonStreamListener extends AsyncResponseListener
3840
* The time that the last message was logged.
3941
*/
4042
protected final AtomicReference<Instant> timeOfLastMessage = new AtomicReference<>(Instant.MIN);
41-
/**
42-
* Lines of text that were returned by the server for logging by the client.
43-
*/
44-
protected final StringBuilder linesToLog = new StringBuilder();
4543

4644
/**
4745
* Creates a new instance.
@@ -136,8 +134,22 @@ protected void processStatus(JsonNode node)
136134
// Only log the status if it's changed or PROGRESS_FREQUENCY has elapsed
137135
lastMessage.set(message);
138136
timeOfLastMessage.set(now);
139-
linesToLog.append(message);
140-
Strings.logLines(linesToLog, log);
137+
log.info(message);
138+
}
139+
}
140+
141+
/**
142+
* @return the server response as JSON
143+
*/
144+
protected JsonNode getResponseBody()
145+
{
146+
try
147+
{
148+
return client.getJsonMapper().readTree(responseAsString.toString());
149+
}
150+
catch (JsonProcessingException e)
151+
{
152+
throw WrappedCheckedException.wrap(e);
141153
}
142154
}
143155
}

src/main/java/com/github/cowwoc/docker/internal/util/Strings.java

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.github.cowwoc.docker.internal.util;
22

3-
import org.slf4j.Logger;
4-
53
import java.text.DecimalFormat;
64
import java.text.DecimalFormatSymbols;
75
import java.text.NumberFormat;
@@ -31,51 +29,6 @@ public static String format(long value)
3129
return FORMATTER.get().format(value);
3230
}
3331

34-
/**
35-
* Logs lines, one at a time.
36-
*
37-
* @param lines zero or more lines of text
38-
* @param log the logger to log to
39-
* @throws NullPointerException if any of the arguments are null
40-
*/
41-
public static void logLines(StringBuilder lines, Logger log)
42-
{
43-
while (true)
44-
{
45-
String line = nextLine(lines);
46-
if (line.isEmpty())
47-
return;
48-
log.info(line);
49-
}
50-
}
51-
52-
/**
53-
* Removes the next line and returns it. If there is only one line left, this method does nothing and
54-
* returns an empty string.
55-
*
56-
* @param lines a {@code StringBuilder} that may contain multiple lines
57-
* @return an empty string if there is only one line left
58-
* @throws NullPointerException if {@code lines} is null
59-
*/
60-
private static String nextLine(StringBuilder lines)
61-
{
62-
int index = lines.indexOf("\n");
63-
if (index != -1)
64-
{
65-
String line = lines.substring(0, index);
66-
lines.delete(0, index + 2);
67-
return line;
68-
}
69-
index = lines.indexOf("\n");
70-
if (index != -1)
71-
{
72-
String line = lines.substring(0, index);
73-
lines.delete(0, index + 1);
74-
return line;
75-
}
76-
return "";
77-
}
78-
7932
private Strings()
8033
{
8134
}

0 commit comments

Comments
 (0)