Skip to content

fix: handle network errors gracefully in token count estimation #437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 30, 2025

Conversation

Napuh
Copy link
Contributor

@Napuh Napuh commented Jul 22, 2025

When downloading tiktoken models fails due to network issues (SSL errors, connection problems), skip token count estimation silently instead of crashing.

This solves a very specific problem I have with an enterprise environment.

@ix-56h
Copy link
Contributor

ix-56h commented Jul 25, 2025

LGTM but we needs to add log when it fails.

@Napuh
Copy link
Contributor Author

Napuh commented Jul 26, 2025

@ix-56h done! I used print because I think that's how logs are managed on the CLI, but lmk if I need to import some other logger.

Copy link
Contributor

@NicolasIRAGNE NicolasIRAGNE left a comment

Choose a reason for hiding this comment

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

Can you revert changes made to changelog? Otherwise lgtm yeah

@ix-56h
Copy link
Contributor

ix-56h commented Jul 26, 2025

Can you revert changes made to changelog? Otherwise lgtm yeah

I've edited the changelog because it markdownlint report an error.
I'll revert it and try to ignore the error for changelog.md only.

@ix-56h
Copy link
Contributor

ix-56h commented Jul 26, 2025

@NicolasIRAGNE done.

@NicolasIRAGNE
Copy link
Contributor

danke, needs a quick rebase and I think we good to go

@Napuh
Copy link
Contributor Author

Napuh commented Jul 30, 2025

Hi!

Is this ok to merge?

@ix-56h ix-56h requested a review from NicolasIRAGNE July 30, 2025 11:31
Copy link
Contributor

@NicolasIRAGNE NicolasIRAGNE left a comment

Choose a reason for hiding this comment

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

aye aye

@NicolasIRAGNE NicolasIRAGNE merged commit 5fbb445 into coderamp-labs:main Jul 30, 2025
41 of 45 checks passed
@Napuh Napuh deleted the no-error-with-networkerror branch July 30, 2025 13:00
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