Skip to content

[LFXV2-1194] fix: handle msgpack-encoded v1-objects KV entries#9

Merged
andrest50 merged 2 commits intomainfrom
fix-encoding
Mar 4, 2026
Merged

[LFXV2-1194] fix: handle msgpack-encoded v1-objects KV entries#9
andrest50 merged 2 commits intomainfrom
fix-encoding

Conversation

@andrest50
Copy link
Contributor

@andrest50 andrest50 commented Mar 4, 2026

Summary

  • Replace single json.Unmarshal call with a two-step approach: try JSON first, fall back to msgpack if JSON fails
  • Add github.com/vmihailenco/msgpack/v5 as a direct dependency
  • Update docs to note that v1 KV entries may be JSON or msgpack encoded

Ticket

LFXV2-1194

v1 KV entries in the v1-objects bucket may be encoded as either JSON or
msgpack. The handler was only attempting JSON deserialization, causing it
to fail silently on msgpack-encoded entries. Add a fallback to msgpack
when JSON deserialization fails, and add github.com/vmihailenco/msgpack/v5
as a dependency.

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Copilot AI review requested due to automatic review settings March 4, 2026 18:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the survey service’s v1 NATS KV event handling to support KV entries encoded as either JSON or msgpack, improving compatibility with legacy v1 data formats.

Changes:

  • Add a JSON-first, msgpack-fallback decoding path for v1 KV entry payloads.
  • Introduce github.com/vmihailenco/msgpack/v5 as a direct dependency (and its transitive go.sum entries).
  • Document that v1 KV entries may be encoded as JSON or msgpack.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
cmd/survey-api/eventing/kv_handler.go Implements JSON→msgpack fallback decoding for v1 KV PUT entries and tweaks prefix extraction.
docs/event-processing.md Documents that v1 KV payloads may be JSON or msgpack and describes the fallback behavior.
go.mod Adds msgpack dependency (and updates direct/indirect requirements).
go.sum Records checksums for the newly added msgpack dependency and transitive modules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…llback

Store entry.Value() once in a local variable to avoid repeated method
calls, and reset v1Data to a fresh map before the msgpack fallback to
prevent stale keys from a partial JSON decode leaking into the result.

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Copy link

@mauriciozanettisalomao mauriciozanettisalomao left a comment

Choose a reason for hiding this comment

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

Approved.

Maybe in the next PR, we can add a revive.toml to remove some lint warnings that we don’t usually address in the code anyway, like package-comments.

@andrest50
Copy link
Contributor Author

Approved.

Maybe in the next PR, we can add a revive.toml to remove some lint warnings that we don’t usually address in the code anyway, like package-comments.

Thanks. Yeah we can have a revive.toml file. The megalinter errors are so widespread and mostly irrelevant that I've let the check fail for a while now. I need to address that in a PR with a megalinter config that works for this codebase.

@andrest50 andrest50 merged commit f05c34e into main Mar 4, 2026
4 of 5 checks passed
@andrest50 andrest50 deleted the fix-encoding branch March 4, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants