Skip to content

Commit bd68f14

Browse files
tboneleedididy
authored andcommitted
[ZEPPELIN-6356] Fix ZeppelinClientIntegrationTest
### What is this PR for? After apache#5090, we started hiding unhandled error details from clients, which caused some tests that depended on those details to fail. The afffected cases are note imports/creations that should fail when the target note path already exists. This PR introduces a `ConflictException` and updates `NotebookRestApi` to return HTTP 409 (Conflict) when `NotebookService` throws or returns `NotePathAlreadyExistsException`. I chose to do this exception-to-HTTP mapping in `NotebookRestApi` because `NotebookService` is also used outside REST contexts, and assigning HTTP status codes belongs in the REST layer. ### What type of PR is it? Bug Fix ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-6356 ### How should this be tested? - Check if `zeppelin-integration-test` and other tests pass ### Screenshots (if appropriate) <img width="1609" height="100" alt="image" src="https://github.com/user-attachments/assets/a445f36c-0267-42df-a75e-9f9f05f94964" /> ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Closes apache#5097 from tbonelee/fix-client-integration-test. Signed-off-by: ChanHo Lee <[email protected]>
1 parent 8982d4f commit bd68f14

File tree

4 files changed

+13
-1
lines changed

4 files changed

+13
-1
lines changed

zeppelin-server/src/main/java/org/apache/zeppelin/rest/AbstractRestApi.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ public void onFailure(Exception ex, ServiceContext context) throws IOException {
5555
super.onFailure(ex, context);
5656
if (ex instanceof WebApplicationException) {
5757
throw (WebApplicationException) ex;
58+
} else if (ex instanceof IOException) {
59+
throw (IOException) ex;
5860
} else {
5961
throw new IOException(ex);
6062
}

zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/WebApplicationExceptionMapper.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import jakarta.ws.rs.ext.ExceptionMapper;
2323
import jakarta.ws.rs.ext.Provider;
2424

25+
import org.apache.zeppelin.notebook.exception.NotePathAlreadyExistsException;
26+
import org.apache.zeppelin.utils.ExceptionUtils;
2527
import org.slf4j.Logger;
2628
import org.slf4j.LoggerFactory;
2729

@@ -34,6 +36,8 @@ public class WebApplicationExceptionMapper implements ExceptionMapper<Throwable>
3436
public Response toResponse(Throwable exception) {
3537
if (exception instanceof WebApplicationException) {
3638
return ((WebApplicationException) exception).getResponse();
39+
} else if (exception instanceof NotePathAlreadyExistsException) {
40+
return ExceptionUtils.jsonResponseContent(Response.Status.CONFLICT, exception.getMessage());
3741
} else {
3842
LOGGER.error("Error response", exception);
3943
// Return generic error message to prevent information disclosure

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,9 @@ public String importNote(String notePath,
385385
callback.onSuccess(note, context);
386386
return note.getId();
387387
});
388-
388+
} catch (NotePathAlreadyExistsException e) {
389+
callback.onFailure(new NotePathAlreadyExistsException("Fail to import note: " + e.getMessage(), e), context);
390+
return null;
389391
} catch (IOException e) {
390392
callback.onFailure(new IOException("Fail to import note: " + e.getMessage(), e), context);
391393
return null;

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/exception/NotePathAlreadyExistsException.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,8 @@ public NotePathAlreadyExistsException(final String message) {
2626
super(message);
2727
}
2828

29+
public NotePathAlreadyExistsException(final String message, final Throwable cause) {
30+
super(message, cause);
31+
}
32+
2933
}

0 commit comments

Comments
 (0)