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

Export error codes #4266

Merged
merged 3 commits into from Oct 7, 2022
Merged

Export error codes #4266

merged 3 commits into from Oct 7, 2022

Conversation

fitiskin
Copy link
Contributor

@fitiskin fitiskin commented Sep 11, 2022

Handling #3842 and #4250.

Checklist

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR! Can you please add a unit test?

@Eomm Eomm linked an issue Sep 24, 2022 that may be closed by this pull request
3 tasks
@Eomm Eomm added the semver-minor Issue or PR that should land as semver minor label Sep 25, 2022
@fitiskin
Copy link
Contributor Author

fitiskin commented Oct 7, 2022

@mcollina Can you please suggest what this unit test should check?

I see a lot of checks for error codes in current test cases like:

t.equal(err.code, 'FST_ERR_SCH_VALIDATION_BUILD')

I can update them to depend on exported errorCodes:

t.ok(err instanceof Fastify.errorCodes.FST_ERR_SCH_VALIDATION_BUILD)

if it makes sense.

I can add new tests but can't find a test case for them since it is just an export.

@fitiskin
Copy link
Contributor Author

fitiskin commented Oct 7, 2022

I see that not all error codes listed in the documentation. I could add missed ones, but it's hard to provide a meaningful description because I am not familiar with them.

@mcollina
Copy link
Member

mcollina commented Oct 7, 2022

I see that not all error codes listed in the documentation. I could add missed ones, but it's hard to provide a meaningful description because I am not familiar with them.

Add them even with a minimum description!

@fitiskin fitiskin marked this pull request as ready for review October 7, 2022 14:49
@fitiskin
Copy link
Contributor Author

fitiskin commented Oct 7, 2022

Add them even with a minimum description!

Done :)

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

(something that would be amazing: automate the maintenance of the error codes).

@mcollina mcollina merged commit 6511ef4 into fastify:main Oct 7, 2022
@fitiskin fitiskin deleted the export-error-codes branch October 8, 2022 14:06
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-minor Issue or PR that should land as semver minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to import fastify error codes?
3 participants