Skip to content

refactor: separate response decoding from emission#554

Merged
lwshang merged 1 commit into
spofford/fix-hexfrom
lwshang/refactor-canister-call-output
May 14, 2026
Merged

refactor: separate response decoding from emission#554
lwshang merged 1 commit into
spofford/fix-hexfrom
lwshang/refactor-canister-call-output

Conversation

@lwshang
Copy link
Copy Markdown
Contributor

@lwshang lwshang commented May 13, 2026

Summary

Proposed addition to #553 — same fix, restructured so the bug class is harder to reintroduce.

Context

In canister call, two flags control output:

  • -o {auto,candid,text,hex} — how to decode the response.
  • --json — wrap the result in a JSON envelope {response_bytes, response_text?, response_candid?}.

They are orthogonal: -o says what decoding to attempt (and whether failure is fatal), --json says how to emit. Previously, each CallOutputMode arm contained its own if args.json { stash } else { print } fork, and a trailing block then serialized the envelope. With four arms × two branches, it was easy for one of the guards to fall out of sync — which is what happened with -o hex --json.

Change

Split the flow into two steps:

  1. decode_response(&res, mode, method) -> Result<Decoded, _> where Decoded ∈ {Candid, Text, Bytes}. This is the only place mode-specific logic lives. Error contexts (response (hex): …, response is not valid UTF-8) are preserved.
  2. A single emission block that forks on args.json exactly once — either build the envelope from Decoded and serialize, or pretty-print to the terminal.

Semantics are unchanged:

  • -o still controls decoding strictness.
  • --json still controls envelope vs. human emission.
  • -o hex --json, -o candid --json, etc. all behave as in fix: Don't print response in -o hex --json #553.
  • The write-vs-decode error reconciliation in JSON mode (decode error wins; write error logged) is preserved.

Each output-mode arm previously forked on `--json`, duplicating the
"decode and stash vs. decode and print" logic. The recent `-o hex --json`
double-printing bug was a direct consequence — easy to forget one of the
`if !args.json` guards.

Split the flow into a `decode_response` step that returns a `Decoded`
enum (Candid / Text / Bytes) and a single emission step that forks on
`--json` exactly once. Semantics are unchanged: `-o` still controls
decoding strictness, `--json` still controls envelope vs. human output,
and the two remain orthogonal. The write-vs-decode error reconciliation
in JSON mode is preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lwshang lwshang changed the title refactor(canister call): separate response decoding from emission refactor: separate response decoding from emission May 13, 2026
@lwshang lwshang marked this pull request as ready for review May 14, 2026 13:43
@lwshang lwshang requested a review from a team as a code owner May 14, 2026 13:43
@lwshang lwshang merged commit 965644c into spofford/fix-hex May 14, 2026
146 of 149 checks passed
@lwshang lwshang deleted the lwshang/refactor-canister-call-output branch May 14, 2026 13:43
lwshang added a commit that referenced this pull request May 14, 2026
* Don't print response in `-o hex --json`

* changelog

* refactor(canister call): separate response decoding from emission (#554)

Each output-mode arm previously forked on `--json`, duplicating the
"decode and stash vs. decode and print" logic. The recent `-o hex --json`
double-printing bug was a direct consequence — easy to forget one of the
`if !args.json` guards.

Split the flow into a `decode_response` step that returns a `Decoded`
enum (Candid / Text / Bytes) and a single emission step that forks on
`--json` exactly once. Semantics are unchanged: `-o` still controls
decoding strictness, `--json` still controls envelope vs. human output,
and the two remain orthogonal. The write-vs-decode error reconciliation
in JSON mode is preserved.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Linwei Shang <linwei.shang@dfinity.org>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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