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

[BUG] Empty HttpEntity results in DeliveryStatus exception #611

Closed
qreshi opened this issue Feb 1, 2023 · 4 comments
Closed

[BUG] Empty HttpEntity results in DeliveryStatus exception #611

qreshi opened this issue Feb 1, 2023 · 4 comments
Assignees
Labels
bug Something isn't working v2.7.0

Comments

@qreshi
Copy link
Contributor

qreshi commented Feb 1, 2023

What is the bug?
The status is retrieved from the response of sendMessage. In this case it'll be calling the execute of DestinationHttpClient to send the message. The responseString here is what becomes the statusText.

The response string is derived from the entity of the response. An HTTP entity in the response can be empty ("") or there can be no entity (null). The null case is being covered by replacing it with the {} string but it looks like the empty case wasn't being covered differently and was just returning an empty string. This is not a valid argument for the statusText param of what gets passed to DeliveryStatus, resulting in an IllegalArgumentException.

What is the expected behavior?
I think we should just check for isNullOrEmpty when getting the response string and make both represented as {}. I only see that method being used to return to the contents of what populates DestinationMessageResponse. If we want to distinguish between null and empty and not use {} for both for whatever reason, then I think DeliveryStatus should support empty statusText if that's derived from the HTTPEntity.

From an HTTP perspective, an empty entity is different from no entity. However, since we're talking about what ultimately becomes the string representation of the statusText, I'd prefer the former approach between the two above as it's simpler.

@qreshi qreshi added bug Something isn't working untriaged and removed untriaged labels Feb 1, 2023
@faabsen
Copy link

faabsen commented Feb 6, 2023

Cheers,

we upgraded some environments to 2.5 but it seems it broke our custom webhook. It’s a very simple one, just a server:port with an extension, nothing complex. We have some environments still at 1.3 and the exact same setup works.

Within the logs we get this error [status_exception] statusText is null or empty

@brijos
Copy link

brijos commented Mar 29, 2023

Another user of OpenSearch project ran into the same issue. The workaround is for the downstream system the webhook is calling needs to return something in the message payload which could be "{}" or "null". This is something which needs to be fixed promptly since http status codes are commonly used. @xluo-aws as a heads up.

@xluo-aws
Copy link
Member

@Hailong,Hailong, could you help take a look at this issue?

@Hailong-am
Copy link
Collaborator

I agree with @qreshi that the string representation of null and an empty HTTPEntity can be the same as {}. If we want to distinguish between them, we could consider replacing the empty value with ok as a possible solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2.7.0
Projects
None yet
Development

No branches or pull requests

6 participants