Skip to content

Commit 6ed85bc

Browse files
m0verm0ver
committed
fix: improve http response handling and connection lifecycle
- ServerResponse: Set response headers to -1 for redirects to signal no body. - ServerResponse: Moved closed state assignment to finally block to ensure consistency. - HttpServer: Added explicit checks for StreamClosedException to prevent redundant error processing. - HttpServer: Improved null safety for error messages and standardized on StandardCharsets.UTF_8. - HTTPHandler: Refactored body reading to handle empty streams gracefully.
1 parent b214bbd commit 6ed85bc

File tree

3 files changed

+95
-62
lines changed

3 files changed

+95
-62
lines changed

‎src/main/java/org/tinystruct/http/ServerResponse.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public void sendRedirect(String url) throws ApplicationException {
5656
addHeader(Header.LOCATION.name(), url);
5757
setStatus(ResponseStatus.TEMPORARY_REDIRECT);
5858
try {
59-
this.exchange.sendResponseHeaders(this.status.code(), 0);
59+
// -1 signals no response body for a redirect
60+
this.exchange.sendResponseHeaders(this.status.code(), -1);
6061
this.headersSent = true;
6162
} catch (IOException e) {
6263
throw new ApplicationException(e);
@@ -86,7 +87,7 @@ public void writeAndFlush(byte[] bytes) throws ApplicationException {
8687
this.outputStream.flush();
8788
}
8889
} catch (IOException e) {
89-
throw new ApplicationException(e.getMessage(), e);
90+
throw new ApplicationException(e.getMessage() != null ? e.getMessage() : e.toString(), e);
9091
}
9192
}
9293

@@ -104,9 +105,9 @@ public void close() throws ApplicationException {
104105
}
105106
} catch (IOException ignore) {
106107
} finally {
108+
closed = true;
107109
exchange.close();
108110
}
109-
closed = true;
110111
}
111112

112113
@Override

‎src/main/java/org/tinystruct/net/handlers/HTTPHandler.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,12 +281,15 @@ public HTTPResponse(HttpResponse<InputStream> response) {
281281
this.headers = response.headers().map();
282282

283283
try (InputStream in = response.body()) {
284-
if (in != null && in.available() > 0) {
284+
if (in != null) {
285285
String contentEncoding = response.headers()
286286
.firstValue("Content-Encoding").orElse(null);
287287
InputStream decodedStream = getDecodedInputStream(contentEncoding, in);
288-
this.body = new String(decodedStream.readAllBytes(), StandardCharsets.UTF_8);
288+
byte[] bytes = decodedStream.readAllBytes();
289289
decodedStream.close();
290+
this.body = bytes.length > 0
291+
? new String(bytes, StandardCharsets.UTF_8)
292+
: null;
290293
} else {
291294
this.body = null;
292295
}

‎src/main/java/org/tinystruct/system/HttpServer.java‎

Lines changed: 86 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -307,20 +307,33 @@ public void handle(HttpExchange exchange) throws IOException {
307307
processRequest(request, response, context);
308308
} catch (ApplicationException e) {
309309
logger.log(Level.SEVERE, e.getMessage(), e);
310+
// If it's a closed stream exception, we shouldn't attempt to write an error back.
311+
if (e.getCause() instanceof IOException && e.getCause().getClass().getName().contains("StreamClosedException")) {
312+
return;
313+
}
314+
310315
int status = e.getStatus();
311316
ResponseStatus responseStatus = ResponseStatus.valueOf(status);
312317
if (responseStatus == null)
313318
responseStatus = ResponseStatus.INTERNAL_SERVER_ERROR;
314319

315320
try {
316-
sendErrorResponse(exchange, responseStatus.code(), e.getMessage());
321+
sendErrorResponse(exchange, responseStatus.code(), e.getMessage() != null ? e.getMessage() : "Error in request processing");
317322
} catch (Exception ignored) {
318323
}
319324
} catch (Exception e) {
320325
logger.log(Level.SEVERE, e.getMessage(), e);
326+
// If it's a closed stream exception, we shouldn't attempt to write an error back.
327+
if (e instanceof IOException && e.getClass().getName().contains("StreamClosedException")) {
328+
return;
329+
}
330+
if (e.getCause() instanceof IOException && e.getCause().getClass().getName().contains("StreamClosedException")) {
331+
return;
332+
}
333+
321334
// Try to send error only if headers haven't been committed yet
322335
try {
323-
sendErrorResponse(exchange, 500, "Internal Server Error: " + e.getMessage());
336+
sendErrorResponse(exchange, 500, "Internal Server Error" + (e.getMessage() != null ? ": " + e.getMessage() : ""));
324337
} catch (Exception ignored) {
325338
// If we can't send an error (headers/body already sent), just log.
326339
}
@@ -354,34 +367,36 @@ private void handleSSE(ServerRequest request, ServerResponse response, Context c
354367
}
355368

356369
Object call = ApplicationManager.call(query, context);
357-
String sessionId = context.getId();
358-
SSEPushManager pushManager = getAppropriatePushManager(isMCP);
359-
response.setStatus(ResponseStatus.OK);
360-
// Ensure chunked streaming for SSE before any write
361-
response.sendHeaders(-1);
362-
SSEClient client = pushManager.register(sessionId, response);
363-
364-
if (call instanceof org.tinystruct.data.component.Builder) {
365-
pushManager.push(sessionId, (org.tinystruct.data.component.Builder) call);
366-
} else if (call instanceof String) {
367-
org.tinystruct.data.component.Builder builder = new org.tinystruct.data.component.Builder();
368-
builder.parse((String) call);
369-
pushManager.push(sessionId, builder);
370-
}
370+
if (!response.isClosed()) {
371+
String sessionId = context.getId();
372+
SSEPushManager pushManager = getAppropriatePushManager(isMCP);
373+
response.setStatus(ResponseStatus.OK);
374+
// Ensure chunked streaming for SSE before any write
375+
response.sendHeaders(-1);
376+
SSEClient client = pushManager.register(sessionId, response);
377+
378+
if (call instanceof org.tinystruct.data.component.Builder) {
379+
pushManager.push(sessionId, (org.tinystruct.data.component.Builder) call);
380+
} else if (call instanceof String) {
381+
org.tinystruct.data.component.Builder builder = new org.tinystruct.data.component.Builder();
382+
builder.parse((String) call);
383+
pushManager.push(sessionId, builder);
384+
}
371385

372-
if (client != null) {
373-
try {
374-
while (client.isActive()) {
375-
Thread.sleep(1000);
386+
if (client != null) {
387+
try {
388+
while (client.isActive()) {
389+
Thread.sleep(1000);
390+
}
391+
} catch (InterruptedException e) {
392+
Thread.currentThread().interrupt();
393+
throw new ApplicationException("Stream interrupted: " + e.getMessage(), e);
394+
} catch (Exception e) {
395+
throw new ApplicationException("Error in stream: " + e.getMessage(), e);
396+
} finally {
397+
client.close();
398+
pushManager.remove(sessionId);
376399
}
377-
} catch (InterruptedException e) {
378-
Thread.currentThread().interrupt();
379-
throw new ApplicationException("Stream interrupted: " + e.getMessage(), e);
380-
} catch (Exception e) {
381-
throw new ApplicationException("Error in stream: " + e.getMessage(), e);
382-
} finally {
383-
client.close();
384-
pushManager.remove(sessionId);
385400
}
386401
}
387402
}
@@ -571,19 +586,26 @@ private void processRequest(ServerRequest request, ServerResponse response, Cont
571586
}
572587
} catch (ApplicationException e) {
573588
logger.log(Level.SEVERE, "Error in request processing", e);
574-
response.setContentType("text/plain; charset=UTF-8");
575-
int status = e.getStatus();
576-
ResponseStatus responseStatus = ResponseStatus.valueOf(status);
577-
if (responseStatus == null)
578-
responseStatus = ResponseStatus.INTERNAL_SERVER_ERROR;
589+
// If it's a closed stream exception, we shouldn't attempt to write an error back.
590+
if (e.getCause() instanceof IOException && e.getCause().getClass().getName().contains("StreamClosedException")) {
591+
return;
592+
}
579593

580-
response.setStatus(responseStatus);
581-
if (e.getMessage() != null) {
582-
response.writeAndFlush(e.getMessage().getBytes(StandardCharsets.UTF_8));
583-
} else {
584-
response.writeAndFlush(new byte[0]);
594+
if (!response.isClosed()) {
595+
try {
596+
response.setContentType("text/plain; charset=UTF-8");
597+
int status = e.getStatus();
598+
ResponseStatus responseStatus = ResponseStatus.valueOf(status);
599+
if (responseStatus == null)
600+
responseStatus = ResponseStatus.INTERNAL_SERVER_ERROR;
601+
602+
response.setStatus(responseStatus);
603+
response.writeAndFlush(e.getMessage() != null ? e.getMessage().getBytes(StandardCharsets.UTF_8) : new byte[0]);
604+
response.close();
605+
} catch (Exception ignored) {
606+
// If we can't send an error (headers/body already sent or stream closed), just ignore.
607+
}
585608
}
586-
response.close();
587609
}
588610
}
589611

@@ -599,22 +621,25 @@ private void handleRequest(String query, Context context, ServerResponse respons
599621
// Handle request
600622
query = StringUtilities.htmlSpecialChars(query);
601623
Object message = ApplicationManager.call(query, context, mode);
602-
byte[] bytes;
603-
if (message != null) {
604-
if (message instanceof byte[]) {
605-
bytes = (byte[]) message;
624+
625+
if (!response.isClosed()) {
626+
byte[] bytes;
627+
if (message != null) {
628+
if (message instanceof byte[]) {
629+
bytes = (byte[]) message;
630+
} else {
631+
response.setContentType("text/html; charset=UTF-8");
632+
bytes = String.valueOf(message).getBytes(StandardCharsets.UTF_8);
633+
}
606634
} else {
607635
response.setContentType("text/html; charset=UTF-8");
608-
bytes = String.valueOf(message).getBytes("UTF-8");
636+
bytes = "No response retrieved!".getBytes(StandardCharsets.UTF_8);
609637
}
610-
} else {
611-
response.setContentType("text/html; charset=UTF-8");
612-
bytes = "No response retrieved!".getBytes("UTF-8");
613-
}
614638

615-
response.setStatus(ResponseStatus.OK);
616-
response.writeAndFlush(bytes);
617-
response.close();
639+
response.setStatus(ResponseStatus.OK);
640+
response.writeAndFlush(bytes);
641+
response.close();
642+
}
618643
}
619644

620645
/**
@@ -625,15 +650,15 @@ private void handleRequest(String query, Context context, ServerResponse respons
625650
* @throws IOException if an I/O error occurs
626651
*/
627652
private void handleDefaultPage(Context context, ServerResponse response) throws ApplicationException {
628-
response.setContentType("text/html; charset=UTF-8");
629653
Object result = ApplicationManager.call(settings.getOrDefault("default.home.page", "say/Praise the Lord."), context, Action.Mode.HTTP_GET);
630654
if (!response.isClosed()) {
631655
try {
632-
byte[] bytes = String.valueOf(result).getBytes("UTF-8");
656+
response.setContentType("text/html; charset=UTF-8");
657+
byte[] bytes = String.valueOf(result).getBytes(StandardCharsets.UTF_8);
633658
response.setStatus(ResponseStatus.OK);
634659
response.writeAndFlush(bytes);
635-
} catch (UnsupportedEncodingException e) {
636-
throw new ApplicationException(e);
660+
} catch (Exception e) {
661+
throw new ApplicationException(e.getMessage(), e);
637662
} finally {
638663
response.close();
639664
}
@@ -642,15 +667,19 @@ private void handleDefaultPage(Context context, ServerResponse response) throws
642667

643668
private void sendErrorResponse(HttpExchange exchange, int statusCode, String message) {
644669
try {
645-
byte[] responseBytes = message.getBytes("UTF-8");
670+
byte[] responseBytes = (message != null ? message : "Unknown error").getBytes(StandardCharsets.UTF_8);
646671
exchange.getResponseHeaders().set("Content-Type", "text/plain; charset=UTF-8");
647672
exchange.sendResponseHeaders(statusCode, responseBytes.length);
648673
try (OutputStream os = exchange.getResponseBody()) {
649674
os.write(responseBytes);
650675
os.flush();
651676
}
652677
} catch (IOException e) {
653-
logger.log(Level.WARNING, "Client disconnected while sending error response", e);
678+
if ("headers already sent".equalsIgnoreCase(e.getMessage())) {
679+
logger.log(Level.FINE, "Headers were already sent; cannot send error response again", e);
680+
} else {
681+
logger.log(Level.WARNING, "Client disconnected while sending error response", e);
682+
}
654683
} finally {
655684
try {
656685
exchange.close();

0 commit comments

Comments
 (0)