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

fix NPE in ScriptedOutgoingMapping when value from script resolved to null #2100

Merged

Conversation

thjaeckle
Copy link
Member

Encountered through failing SystemTests on master branch:
https://github.com/eclipse-ditto/ditto/runs/35877513588#r25

@thjaeckle thjaeckle added the bug label Jan 20, 2025
@thjaeckle thjaeckle added this to the 3.6.10 milestone Jan 20, 2025
@thjaeckle thjaeckle self-assigned this Jan 20, 2025
@thjaeckle
Copy link
Member Author

Down from 6 Test failures to 4:
https://github.com/eclipse-ditto/ditto/runs/35883171669#r32

Will investigate further why the 4 remain ..

@thjaeckle
Copy link
Member Author

It seems like message headers for AMQP 1.0 and RabbitMQ are somehow lowercased.

Before the integration test ensureSourceAuthenticationPlaceholderSubstitution() did send a header named headerId - and used that header in a placeholder substitution.
Running the test locally I found out that this header is received at Ditto as headerid so the capital I gets lowercases to i.
I found the same for messageId header - lowercased to messageid.

For MQTT5 e.g. this test passes, so case is preserved there.

@alstanchev do you have any idea why this could happen?
Maybe a dependency update?

@alstanchev
Copy link
Contributor

alstanchev commented Jan 20, 2025

That is strange.
No idea, doesn't ring any bells.

@thjaeckle
Copy link
Member Author

@alstanchev I found it .. a DittoHeadersBuilder lowercases by default all read external header keys.
And I used DittoHeadersBuilder to remove "traceparent" header in order to fix the trace span-hierarchy.

Fixed in another commit to this PR.

If you could have a look, that would be great.
I will also run system tests again.

@thjaeckle
Copy link
Member Author

System tests are green:
https://github.com/eclipse-ditto/ditto/actions/runs/12882019337

@thjaeckle thjaeckle requested a review from alstanchev January 21, 2025 08:43
Copy link
Contributor

@alstanchev alstanchev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thjaeckle thjaeckle merged commit 4966ef4 into eclipse-ditto:master Jan 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants