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.

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

[websocket] WebSocket proxy should return status code depending on type of PulsarClientException #9031

Merged
merged 2 commits into from
Dec 24, 2020

Conversation

massakam
Copy link
Contributor

Motivation

If the WebSocket proxy fails to create a producer or consumer, it should return the suitable HTTP status code to the client depending on the type of PulsarClientException that occurred. However, there are currently only a few exceptions where the WebSocket proxy returns a status code other than 500.

private static int getErrorCode(Exception e) {
if (e instanceof IllegalArgumentException) {
return HttpServletResponse.SC_BAD_REQUEST;
} else if (e instanceof ProducerBusyException) {
return HttpServletResponse.SC_CONFLICT;
} else if (e instanceof ProducerBlockedQuotaExceededError || e instanceof ProducerBlockedQuotaExceededException) {
return HttpServletResponse.SC_SERVICE_UNAVAILABLE;
} else {
return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
}
}

Modifications

Moved the getErrorCode() method from ProducerHandler and ConsumerHandler to AbstractWebSocketHandler and increased the types of PulsarClientException to handle.

@massakam massakam self-assigned this Dec 23, 2020
@massakam massakam added this to the 2.8.0 milestone Dec 23, 2020
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me. Could you please also some tests for the new change?

@sijie sijie added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Dec 23, 2020
@massakam
Copy link
Contributor Author

@codelipenghui It's hard to cover all cases, so added tests for some of them.

@massakam massakam changed the title WebSocket proxy should return status code depending on type of PulsarClientException [websocket] WebSocket proxy should return status code depending on type of PulsarClientException Dec 24, 2020
@sijie sijie merged commit 8c45b82 into apache:master Dec 24, 2020
@massakam massakam deleted the ws-handle-exception branch December 25, 2020 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/websocket type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants