Skip to content

refactor: default to OTP json, drop thoas dependency#386

Open
Taure wants to merge 1 commit into
masterfrom
feat/json-default-otp
Open

refactor: default to OTP json, drop thoas dependency#386
Taure wants to merge 1 commit into
masterfrom
feat/json-default-otp

Conversation

@Taure
Copy link
Copy Markdown
Collaborator

@Taure Taure commented May 19, 2026

Summary

  • Switches the default json_lib from thoas to the OTP json module and removes thoas from the dependency list.
  • json_lib remains configurable so users can plug in any module that follows the OTP contract: encode/1 -> binary() | iodata(), decode/1 -> Term (raises on bad input).
  • nova_request_plugin wraps decode/1 in try/catch to keep returning 400 on malformed JSON.
  • nova_jsonlogger:encode/2 wraps its output in iolist_to_binary/1 so the formatter still returns a binary regardless of backend.
  • Inline jsonlogger tests and the configuration guide updated to the OTP json contract.

Test plan

  • rebar3 compile
  • rebar3 xref
  • rebar3 dialyzer
  • rebar3 eunit (8/0 — same as master baseline)

@Taure Taure force-pushed the feat/json-default-otp branch from f68af61 to ceec233 Compare May 19, 2026 21:07
Switch the default json_lib from thoas to the OTP json module and remove
thoas from the dependency list. The json_lib config remains so users can
plug in any module that follows the OTP contract:
encode/1 -> binary() | iodata(), decode/1 -> Term (raises on bad input).

- nova_request_plugin: wrap decode/1 in try/catch to keep 400 responses
  on malformed JSON.
- nova_jsonlogger: wrap encode/2 output in iolist_to_binary/1 so the
  formatter still returns a binary regardless of backend.
- Update inline tests to the OTP json contract (no {ok, _} wrapper).
- Update configuration guide to document the new default and contract.
@Taure Taure force-pushed the feat/json-default-otp branch from ceec233 to 6b13d49 Compare May 20, 2026 05:01
@Taure Taure requested a review from burbas May 26, 2026 18:39
Copy link
Copy Markdown
Contributor

@burbas burbas left a comment

Choose a reason for hiding this comment

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

We need to have a discussion when to use try..catch for these kinds of operations and when not to.

{ok, JSON} ->
modulate_state(Req#{json => JSON}, Tl, State);
Error ->
JsonLib = nova:get_env(json_lib, json),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I add my comment to this block since it showcase my point;

Here we do try...catch but in the rest of the code we just let it crash. That we can not have - we need to discuss a strategy on how to handle these kinds of problems. Do we crash or do we catch it? It might be that we want to do different things in different situations - but think we need to discuss this more broadly.

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.

2 participants