Skip to content

fix: helpful error when external returns a non-cloneable value - #149

Merged
KnorpelSenf merged 2 commits into
grammyjs:mainfrom
Dramex:fix/external-structured-clone-error
May 16, 2026
Merged

fix: helpful error when external returns a non-cloneable value#149
KnorpelSenf merged 2 commits into
grammyjs:mainfrom
Dramex:fix/external-structured-clone-error

Conversation

@Dramex

@Dramex Dramex commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #140.

conversation.external(...) clones its result with structuredClone so the value is immutable across replays. When the task returns a non-cloneable value (for example, a function), the native DataCloneError that falls out doesn't mention external, the conversation, or what to do about it — the reporter on #140 hit exactly this.

Wrap the structuredClone(ret) call in src/conversation.ts so that failures throw a new Error that:

  • Names external as the site where cloning is required.
  • Suggests beforeStore / afterLoad for custom serialization (already documented in the JSDoc above).
  • Preserves the original DataCloneError on .cause for debugging.

Changes

  • src/conversation.ts: try/catch around structuredClone(ret), rethrow with a helpful message and cause.
  • test/conversation.test.ts: new case exercising conversation.external(() => () => 0) and asserting the new message and cause.

Test plan

  • deno task test --no-check — 11 suites / 117 steps pass (the repo has pre-existing RequestInit.body type errors in test/conversation.test.ts:390,525 that reproduce on main without this change).
  • deno fmt --check
  • deno lint

@KnorpelSenf KnorpelSenf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Solid stuff

@KnorpelSenf

Copy link
Copy Markdown
Member

@Dramex I have fixed CI upstream, please update your branch

@KnorpelSenf

Copy link
Copy Markdown
Member

Also: I have seen all of your other amazing contributions, and I am sorry for ghosting your for so long. This is probably the first time since the inception of grammY that I am this unresponsive for this long, there is just too much going on in my life right now (only good things). I am very much looking forward to merging them soon, thanks for your patience!

I am still present in https://t.me/grammyjs if you ever want to have a chat about anything

`external` runs the caller's task outside the deterministic replay and
then clones the result via `structuredClone` so the returned value is
immutable across replays. When the task returns something that
`structuredClone` cannot handle (e.g. a function), the clone throws a
`DataCloneError` whose message does not mention `external`, the
conversation, or the fix.

Wrap the `structuredClone` call so that such failures raise a new
`Error` that points at `external`, suggests `beforeStore`/`afterLoad`
for custom serialisation, and keeps the original `DataCloneError` as
its `cause` for debugging.

Closes grammyjs#140.
@Dramex
Dramex force-pushed the fix/external-structured-clone-error branch from 72fd7c9 to deee00a Compare May 15, 2026 15:09
@Dramex

Dramex commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

rebased onto main. and no worries, life first.

Comment thread src/conversation.ts
@KnorpelSenf
KnorpelSenf merged commit 863e8e5 into grammyjs:main May 16, 2026
6 checks passed
@KnorpelSenf

Copy link
Copy Markdown
Member

TYSM!

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.

feat: improve error message for failed structuredClone calls

2 participants