Skip to content

feat: expose error type#276

Merged
bgub merged 7 commits into
bgub:mainfrom
multivoltage:feature/expose-error-type
Mar 18, 2024
Merged

feat: expose error type#276
bgub merged 7 commits into
bgub:mainfrom
multivoltage:feature/expose-error-type

Conversation

@multivoltage
Copy link
Copy Markdown
Contributor

I tried to export EtaError types introducing new types like @nebrelbug did in THIS issue.

Some point about he PR:

  • I put an EtaXXXError instead EtaError in all point. So at the and I asked myself "Is now useless EtaError type? Can I remove it"
  • I added a test for each error interface. I tried to throw starting from top-level api. For example using .render() even if the exception is inside .readFile()
  • I added never as return type since for me a function that throw 100% of times an error equals to a function that never run an error (token from TS docs)

I don't know if this PR follow exactly the structure described here:
#243 (comment)

But probably this PR can be a starting point and I'm happy to change some part after review

@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 11, 2024

@multivoltage thanks for the PR! This looks great. Just a few changes before I can merge:

  • Rename EtaRuntimeErr to EtaRuntimeError
  • I think it would be great if all Eta errors extended a base class EtaError, which could just be declared like class EtaError extends Error {}.

I appreciate the TypeScript updates!

@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 11, 2024

Also if you can make sure to follow the Commitlint format for commit messages, that's what's causing tests to fail right now. Thanks!!

@multivoltage
Copy link
Copy Markdown
Contributor Author

npx commitlint --from ee0a1d44addd162b53b20329799b93628fcba22e~6 --to ee0a1d44addd162b53b20329799b93628fcba22e --verbose
⧗   input: feat: export EtaNameResolutionError
✔   found 0 problems, 0 warnings
⧗   input: feat: add use case for EtaFileResolutionError
✔   found 0 problems, 0 warnings
⧗   input: feat: export EtaFileResolutionError
✔   found 0 problems, 0 warnings
⧗   input: feat: use custom error for compile() api
✔   found 0 problems, 0 warnings
⧗   input: feat: export EtaRuntimeError
✔   found 0 problems, 0 warnings
⧗   input: feat: export EtaParseError
✔   found 0 problems, 0 warnings

In my local pc seems ok (node 18)

@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 12, 2024

@multivoltage hmm, you're right. I'm not actually sure what the problem is, but it seems like it may be a problem with the version of commitlint that the GitHub Action is installing.

@multivoltage
Copy link
Copy Markdown
Contributor Author

@nebrelbug I replicated issue.
In. my local pc like I said I run "npx commitlint....".

But in you yaml you install before

      - name: Install commitlint
        run: |
          npm install conventional-changelog-conventionalcommits
          npm install commitlint@latest

and then use npx.

I think you have 2 possible fix:

  1. remove step to install the two deps and keep going to use "npx commitlint" like in my local pc.
  2. Installing same deps in my pc and run "npx commitlint" the error is the same. I solved running also @commitlint/format

Please give me a feedback. Do you plan to fix this in seperate pr?

@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 16, 2024

@multivoltage I'll fix the commitlint issue in a separate PR later. In the meantime, I'll merge anyways if you can fix the issues I mentioned above.

@multivoltage
Copy link
Copy Markdown
Contributor Author

@nebrelbug
Sorry. I did not notice your first comment :)

I think it would be great if all Eta errors extended a base class EtaError, which could just be declared like class EtaError extends Error {}.

I thought the same thing inirtially but then I ask my self?

What EtaError give me more than a basic Erro? Anyway I will do it :)

@multivoltage multivoltage force-pushed the feature/expose-error-type branch from ee0a1d4 to e4e8ebb Compare March 17, 2024 18:20
@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 18, 2024

@multivoltage thank you! Having a base EtaError class will be useful if people want to catch all errors that Eta throws.

@bgub bgub changed the title Feature/expose error type feat: expose error type Mar 18, 2024
@bgub bgub merged commit 70bd7e3 into bgub:main Mar 18, 2024
@bgub
Copy link
Copy Markdown
Owner

bgub commented Mar 18, 2024

@multivoltage released in Eta 3.4.0:)

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