Skip to content

Improve internal error reporting guidance#2345

Merged
jcubic merged 1 commit into
isomorphic-git:mainfrom
Will-thom:will/isomorphic-git-2216-fatal-message
May 26, 2026
Merged

Improve internal error reporting guidance#2345
jcubic merged 1 commit into
isomorphic-git:mainfrom
Will-thom:will/isomorphic-git-2216-fatal-message

Conversation

@Will-thom

@Will-thom Will-thom commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • clarify InternalError guidance for application users vs developers
  • keep isomorphic-git issue reporting guidance targeted to developers with reproduction details
  • add error-message coverage in GitError tests

Closes #2216

Tests

  • npx nps lint
  • npx prettier --check src/errors/InternalError.js tests/test-GitError.js tests/test-GitError-in-submodule.js
  • $env:NODE_OPTIONS='--experimental-vm-modules --max-old-space-size-percentage=80'; npx jest tests/test-GitError.js tests/test-GitError-in-submodule.js --runInBand --ci --coverage=false
  • npx nps test.typecheck

Summary by CodeRabbit

  • Bug Fixes

    • Improved error messages with clearer, actionable guidance for users regarding issue reporting and for developers regarding required information for bug reports, including version, environment details, and reproduction steps.
  • Tests

    • Added test cases to verify error message formatting and ensure proper guidance text is included for both end-users and developers.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 65d0de57-5ef4-4152-a886-e095ca334aed

📥 Commits

Reviewing files that changed from the base of the PR and between 89f441d and 16e4eb1.

📒 Files selected for processing (3)
  • __tests__/test-GitError-in-submodule.js
  • __tests__/test-GitError.js
  • src/errors/InternalError.js

📝 Walkthrough

Walkthrough

This PR improves the InternalError message to provide clearer guidance for reporting bugs. The error message now directs non-developers to report issues to their application developers and instructs developers to file issues in isomorphic-git with proper reproduction details, version, and environment information. Two test files validate the updated message content.

Changes

InternalError messaging improvement

Layer / File(s) Summary
InternalError message and validation tests
src/errors/InternalError.js, __tests__/test-GitError.js, __tests__/test-GitError-in-submodule.js
InternalError message text is updated to include actionable guidance for users versus developers on reporting bugs properly. Unit tests in both main and submodule test suites verify the new message includes the expected guidance strings and original error text.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 An error's voice, once unclear and light,
Now guides both users and developers right—
Report upstream if not a coder true,
But devs file issues with details new.
A message kind makes debugging bright! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Improve internal error reporting guidance' directly and clearly summarizes the main change of updating InternalError message guidance for different user types.
Linked Issues check ✅ Passed The PR implementation meets all coding requirements from issue #2216: InternalError message now differentiates guidance for non-developers vs. developers, includes reproduction/version/environment details for isomorphic-git reporting, and comprehensive test coverage validates the new message content.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the PR objectives: only InternalError.js message and related test coverage were modified; no unrelated files or functionality were altered.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jcubic jcubic merged commit 955acf3 into isomorphic-git:main May 26, 2026
5 checks passed
@jcubic

jcubic commented May 26, 2026

Copy link
Copy Markdown
Member

Thanks for improving the error message.

@jcubic

jcubic commented May 26, 2026

Copy link
Copy Markdown
Member

The fix to that issue was actually already applied. The error messages already mention the application that use isomorphic-git.

The old message (that was in the issue) only had CTA to report a bug in isomorphic-git. Because of this, when one app had a bug, we had a flood of bug reports.

But it's always good to mention reproduction.

@jcubic

jcubic commented May 26, 2026

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.38.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve fatal error message

2 participants