Skip to content

Commit 88e8cf5

Browse files
committed
feat: add error handling for GetLedgerMetaService
Wrap LedgerManager.readLedgerMetadata() call in try-catch-finally to properly handle errors and ensure manager is always closed: - BKNoSuchLedgerExistsOnMetadataServerException → 404 NOT_FOUND - Other BK errors → 500 INTERNAL_ERROR with error message Add testGetLedgerMetaServiceNotFound to verify non-existent ledger returns 404 with expected error body.
1 parent 978602c commit 88e8cf5

2 files changed

Lines changed: 41 additions & 12 deletions

File tree

bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/GetLedgerMetaService.java

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
import com.google.common.collect.Maps;
2424
import java.util.Map;
25+
import java.util.concurrent.ExecutionException;
26+
import org.apache.bookkeeper.client.BKException;
2527
import org.apache.bookkeeper.client.api.LedgerMetadata;
2628
import org.apache.bookkeeper.common.util.JsonUtil;
2729
import org.apache.bookkeeper.conf.ServerConfiguration;
@@ -63,20 +65,31 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
6365
Long ledgerId = Long.parseLong(params.get("ledger_id"));
6466

6567
LedgerManager manager = ledgerManagerFactory.newLedgerManager();
68+
try {
69+
// output <ledgerId: ledgerMetadata>
70+
Map<String, Object> output = Maps.newHashMap();
71+
LedgerMetadata md = manager.readLedgerMetadata(ledgerId).get().getValue();
72+
output.put(ledgerId.toString(), md);
6673

67-
// output <ledgerId: ledgerMetadata>
68-
Map<String, Object> output = Maps.newHashMap();
69-
LedgerMetadata md = manager.readLedgerMetadata(ledgerId).get().getValue();
70-
output.put(ledgerId.toString(), md);
71-
72-
manager.close();
73-
74-
String jsonResponse = JsonUtil.toJson(output);
75-
if (LOG.isDebugEnabled()) {
76-
LOG.debug("output body:" + jsonResponse);
74+
String jsonResponse = JsonUtil.toJson(output);
75+
if (LOG.isDebugEnabled()) {
76+
LOG.debug("output body:" + jsonResponse);
77+
}
78+
response.setBody(jsonResponse);
79+
response.setCode(HttpServer.StatusCode.OK);
80+
} catch (ExecutionException e) {
81+
if (e.getCause() instanceof BKException.BKNoSuchLedgerExistsOnMetadataServerException) {
82+
LOG.debug("Ledger {} not found on metadata server", ledgerId);
83+
response.setCode(HttpServer.StatusCode.NOT_FOUND);
84+
response.setBody("Ledger " + ledgerId + " not found on metadata server");
85+
} else {
86+
LOG.error("Failed to read ledger metadata for {}", ledgerId, e.getCause());
87+
response.setCode(HttpServer.StatusCode.INTERNAL_ERROR);
88+
response.setBody("Failed to read ledger metadata: " + e.getCause().getMessage());
89+
}
90+
} finally {
91+
manager.close();
7792
}
78-
response.setBody(jsonResponse);
79-
response.setCode(HttpServer.StatusCode.OK);
8093
return response;
8194
} else {
8295
response.setCode(HttpServer.StatusCode.NOT_FOUND);

bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/TestHttpService.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,22 @@ public void testGetLedgerMetaService() throws Exception {
414414
assertTrue(Maps.difference(expected, actual).areEqual());
415415
}
416416

417+
@Test
418+
public void testGetLedgerMetaServiceNotFound() throws Exception {
419+
baseConf.setMetadataServiceUri(zkUtil.getMetadataServiceUri());
420+
HttpEndpointService getLedgerMetaService = bkHttpServiceProvider
421+
.provideHttpEndpointService(HttpServer.ApiType.GET_LEDGER_META);
422+
423+
// query a non-existent ledger, should return NOT_FOUND
424+
HashMap<String, String> params = Maps.newHashMap();
425+
long nonExistentLedgerId = 99999999L;
426+
params.put("ledger_id", Long.toString(nonExistentLedgerId));
427+
HttpServiceRequest request = new HttpServiceRequest(null, HttpServer.Method.GET, params);
428+
HttpServiceResponse response = getLedgerMetaService.handle(request);
429+
assertEquals(HttpServer.StatusCode.NOT_FOUND.getValue(), response.getStatusCode());
430+
assertTrue(response.getBody().contains("not found on metadata server"));
431+
}
432+
417433
@Test
418434
public void testReadLedgerEntryService() throws Exception {
419435
BookKeeper.DigestType digestType = BookKeeper.DigestType.CRC32;

0 commit comments

Comments
 (0)