camunda / connectors

Camunda Connectors
https://docs.camunda.io/docs/components/integration-framework/connectors/out-of-the-box-connectors/available-connectors-overview/
Apache License 2.0
35 stars 36 forks source link

RabbitMQ Connector does unnecessary message unsescape action. #2730

Closed Zapolski closed 2 weeks ago

Zapolski commented 1 month ago

Describe the Bug

I use out-of-the-box RabbitMQ Connector. Message for sending contains string fields with escaped symbols, like:

{
   "data": {
       "company": "LTD \"Three bears\""
   }
}

Connector sends my message but consumer receives wrong message and fells with MessageConversionException. There’s a message which consumer receives:

{
   "data": {
       "company": "LTD "Three bears""
   }
}

All symbols "\" disappeared. Connector’s source code contains logic with unescaping json.

Steps to Reproduce

  1. Prepare message object for sending by RabbitMQ connector. Object must contain field with escaped symbols. For example:
    {
    "data": {
       "company": "LTD \"Three bears\""
    }
    }
  2. Use RabbitMQ connector for sending message.

Expected Behavior

I expect the same message in my RabbitMQ CONSUMER, message should be without any transformation.

Environment

Camunda 8 forum topic

sbuettner commented 3 weeks ago

As discussed in our team meeting we will remove unnecessary unescaping from Connectors in the next alpha.

mathias-vandaele commented 2 weeks ago

After testing, removing .map(StringEscapeUtils::unescapeJson) L65 io.camunda.connector.rabbitmq.outbound.MessageUtil corrects the issue. I am not so sure about removing it as it might have been added for a valid reason, any thoughts @sbuettner ?

johnBgood commented 2 weeks ago

I'm posting the forum comment I made here for visibility:

It is due to Zeebe escaping characters. Looking at this ticket, it seems that it might be resolved. If so, we will remove the unescapes in our connectors.

The way I see it, we might remove any unescaping (not only in the RabbitMQ connector), but let's wait for @sbuettner or @chillleader opinions.

chillleader commented 2 weeks ago

Right, that Zeebe issue was the reason we added it there. If the escaping issues are gone, we no longer need the workaround.

mathias-vandaele commented 2 weeks ago

@chillleader Should I make an update and modify it in every connector ?

chillleader commented 2 weeks ago

Yes please - although I imagine there might be some corner cases where we might decide to keep unescaping for certain reasons (or maybe not, let's see during the review 🙂). But in general I think we should remove it from all connectors now.

sbuettner commented 2 weeks ago

Yes, lets remove unnecessary escaping but not backport it as previous zeebe releases faced this issue.