Skip to content
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

Add custom message to _unsafeUnwrap* #590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmmulroy
Copy link
Contributor

@dmmulroy dmmulroy commented Oct 9, 2024

Pull Request Description

Summary

This PR introduces enhancements to the error handling mechanism within the codebase by allowing custom error messages to be specified. Additionally, it updates the README documentation and adds new test cases.

Changes Made

  1. README.md:

    • Added documentation on how to conditionally add a custom error message that will propagate with the error object.
    • Provided an example to demonstrate the usage of the custom error message with a stack trace.
  2. src/_internals/error.ts:

    • Updated the ErrorConfig interface to include an optional message property.
  3. src/result.ts:

    • Modified the _unsafeUnwrapErr method in the Ok class to use the custom error message if provided, otherwise, it defaults to the existing message.
    • Updated the _unsafeUnwrap method in the Err class to use the custom error message if provided, otherwise, it defaults to the existing message.
  4. tests/index.test.ts:

    • Added test cases to verify that custom error messages are correctly included when _unsafeUnwrap and _unsafeUnwrapErr are called with a custom message in the Ok and Err classes.

Copy link

changeset-bot bot commented Oct 9, 2024

🦋 Changeset detected

Latest commit: 72e0a7c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
neverthrow Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dmmulroy dmmulroy force-pushed the dmmulroy/unwrap-message branch from 654c398 to 72e0a7c Compare October 9, 2024 13:16
@dmmulroy
Copy link
Contributor Author

dmmulroy commented Oct 9, 2024

@supermacro rebased on to master and added a changeset. This is the same work as #558

@supermacro
Copy link
Owner

safe to close this, right?

Copy link
Collaborator

@m-shaka m-shaka 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 resolve conflicts and revert formatter change of index.test.ts?

I know we should format and commit test files too and check them on CI, but before doing it prettier must be upgraded. I'm thinking replace prettier with biome instead, though

@dmmulroy
Copy link
Contributor Author

Ope yup sorry, this notification got buried - will update tomorrow!

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.

4 participants