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

Sending a message with metadata or headers doesn't work #193

Closed
lawrence-forooghian opened this issue Dec 12, 2024 · 0 comments · Fixed by #196
Closed

Sending a message with metadata or headers doesn't work #193

lawrence-forooghian opened this issue Dec 12, 2024 · 0 comments · Fixed by #196
Assignees
Labels
bug Something isn’t working. It’s clear that this does need to be fixed.

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Dec 12, 2024

In the integration tests, try adding the following to a call to room.messages.send:

metadata: ["hello": .number(12), "goodbye": .string("what")]

You'll see that the call to send fails with noItemInResponse, and in the logs we see

Could not messagepack object: AblyChat.MetadataValue.number(12)
Could not messagepack object: AblyChat.MetadataValue.string("what")
WARN: (ARTHttp.m:57) unable to publish chat message; cannot decode request body

Similarly, with headers:

headers: ["hello": .number(12), "goodbye": .string("what")]

we get

Could not messagepack object: AblyChat.HeadersValue.string("what")
Could not messagepack object: AblyChat.HeadersValue.number(12)
WARN: (ARTHttp.m:57) unable to publish chat message; cannot decode request body

┆Issue is synchronized with this Jira Task by Unito

@lawrence-forooghian lawrence-forooghian added the bug Something isn’t working. It’s clear that this does need to be fixed. label Dec 12, 2024
@lawrence-forooghian lawrence-forooghian self-assigned this Dec 12, 2024
@lawrence-forooghian lawrence-forooghian changed the title Sending a message with metadata doesn't work Sending a message with metadata or headers doesn't work Dec 12, 2024
lawrence-forooghian added a commit that referenced this issue Dec 12, 2024
We were not serializing or deserializing the Chat SDK’s Metadata and
Headers types. This was a mistake in 8b6e56a.

I was surprised to find that there were no existing tests for the
request that ChatAPI makes when you call sendMessage, nor for
DefaultMessages’s handling of messages received over a realtime channel,
which meant that there wasn’t an obvious place to slot in the tests for
the fixes introduced by this commit, nor did the mocks have any support
for writing such tests. I’ve added tests for my fixes but, given my rush
to get this fix done, the changes to the mocks aren’t great. Have
created issue #195 for us come back and write the missing tests and tidy
mine up.

Note that I’ve removed the optionality of Metadata’s values. This
optionality came from 20e7f5f when it was unclear what Metadata would
be. We probably should have removed it in 8b6e56a when we introduced
`null` as a MetadataValue case.

Resolves #193.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working. It’s clear that this does need to be fixed.
Development

Successfully merging a pull request may close this issue.

1 participant