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

ContentTypeParser.remove, does not throw if parser does not exist #4482

Closed
2 tasks done
Uzlopak opened this issue Dec 24, 2022 · 4 comments · Fixed by #4550
Closed
2 tasks done

ContentTypeParser.remove, does not throw if parser does not exist #4482

Uzlopak opened this issue Dec 24, 2022 · 4 comments · Fixed by #4550
Labels
bug Confirmed bug good first issue Good for newcomers

Comments

@Uzlopak
Copy link
Contributor

Uzlopak commented Dec 24, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the issue has not already been raised

Issue

I find it quite inconsequent, if the remove method of ContentTypeParser in case it is not a string or RegExp throws a FST_ERR_CTP_INVALID_TYPE-error but not an Error if there is no parser matching the contentType.

Imho the else case of this if-condition should throw an Error

if (idx > -1) {

@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers labels Dec 24, 2022
@climba03003
Copy link
Member

climba03003 commented Dec 24, 2022

I am not sure if it should be a bug or something that should fix.
Does anyone really care about if the content-type that should be remove is not exist? If not, throwing shouldn't be a good DX either.

@RajPabnani03
Copy link

@climba03003 Yes it Matters if the content-type is available or through error when the value on removal is not equal to NULL !!!

@climba03003
Copy link
Member

You say it matters.
But I don't see the later part of the comment show how does it matter.

@kamilogorek
Copy link
Contributor

It's really hard to spot the typo in the content type parser that you want to remove, as the input there is a raw string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug good first issue Good for newcomers
Projects
None yet
5 participants