Skip to content

Conversation

@hobostay
Copy link
Contributor

Summary

  • Fixed a bug where get_tokenizer() would panic instead of returning a proper error when tokenizer initialization fails
  • Changed the function to properly return Result types as intended by its signature

Details

The get_tokenizer() function in crates/goose/src/token_counter.rs returns Result<Arc<CoreBPE>, String>, but was using panic!() when the tiktoken_rs::o200k_base() call failed. This would crash the program instead of gracefully handling the error.

Changes made:

  1. Updated TOKENIZER static from OnceCell<Arc<CoreBPE>> to OnceCell<Result<Arc<CoreBPE>, String>>
  2. Modified the error handling in the initializer closure to return Err(...) instead of panic!(...)
  3. Updated the return statement to clone and return the Result directly

Test plan

  • All existing tests pass
  • Code compiles without warnings
  • Verified error propagation works correctly

🤖 Generated with Claude Code

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

thanks. this had been bugging me for a long time!

@DOsinga
Copy link
Collaborator

DOsinga commented Feb 12, 2026

can you do the DCO thing so we can get this merged?

The get_tokenizer() function returns Result<Arc<CoreBPE>, String> but was
using panic!() when tokenizer initialization failed, which would crash the
program instead of returning an error.

Changed:
- Updated TOKENIZER static to store Result<Arc<CoreBPE>, String>
- Modified get_tokenizer() to return Err() instead of panic!() on failure
- This allows proper error propagation instead of causing a crash

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: hobostay <110hqc@gmail.com>
@hobostay hobostay force-pushed the fix/panic-to-error-in-tokenizer branch from 3cec137 to 463aced Compare February 12, 2026 10:26
@hobostay
Copy link
Contributor Author

Done! I've added the Signed-off-by line to satisfy the DCO check.

@DOsinga DOsinga added this pull request to the merge queue Feb 12, 2026
Merged via the queue into block:main with commit a82a1d7 Feb 12, 2026
19 checks passed
jh-block added a commit that referenced this pull request Feb 12, 2026
* origin/main: (33 commits)
  fix: replace panic with proper error handling in get_tokenizer (#7175)
  Lifei/smoke test for developer (#7174)
  fix text editor view broken (#7167)
  docs: White label guide (#6857)
  Add PATH detection back to developer extension (#7161)
  docs: pin version in ci/cd (#7168)
  Desktop: - No Custom Headers field for custom OpenAI-compatible providers  (#6681)
  feat: edit model and extensions of a recipe from GUI (#6804)
  feat: MCP support for agentic CLI providers (#6972)
  docs: keyring fallback to secrets.yaml (#7165)
  feat: load provider/model specified inside the recipe config (#6884)
  fix ask-ai bot hitting tool call limits (#7162)
  fix flatpak icon (#7154)
  [docs] Skills Marketplace UI Improvements (#7158)
  More no-window flags (#7122)
  feat: Allow overriding default bat themes using environment variables (#7140)
  Make the system prompt smaller (#6991)
  Pre release script (#7145)
  Spelling (#7137)
  feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (#6927)
  ...
tlongwell-block added a commit that referenced this pull request Feb 12, 2026
…provenance

* origin/main: (68 commits)
  Upgraded npm packages for latest security updates (#7183)
  docs: reasoning effort levels for Codex provider (#6798)
  Fix speech local (#7181)
  chore: add .gooseignore to .gitignore (#6826)
  Improve error message logging from electron (#7130)
  chore(deps): bump jsonwebtoken from 9.3.1 to 10.3.0 (#6924)
  docs: standalone mcp apps and apps extension (#6791)
  workflow: auto-update cli-commands on release (#6755)
  feat(apps): Integrate AppRenderer from @mcp-ui/client SDK (#7013)
  fix(MCP): decode resource content (#7155)
  feat: reasoning_content in API for reasoning models (#6322)
  Fix/configure add provider custom headers (#7157)
  fix: handle keyring fallback as success (#7177)
  Update process-wrap to 9.0.3 (9.0.2 is yanked) (#7176)
  feat: support extra field in chatcompletion tool_calls for gemini openai compat (#6184)
  fix: replace panic with proper error handling in get_tokenizer (#7175)
  Lifei/smoke test for developer (#7174)
  fix text editor view broken (#7167)
  docs: White label guide (#6857)
  Add PATH detection back to developer extension (#7161)
  ...

# Conflicts:
#	.github/workflows/nightly.yml
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