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 code and status properties to produced errors #337

Closed
stefanblaginov opened this issue Sep 24, 2021 · 13 comments
Closed

Add code and status properties to produced errors #337

stefanblaginov opened this issue Sep 24, 2021 · 13 comments
Labels

Comments

@stefanblaginov
Copy link
Contributor

Feature Request 🛍️

Add code and status properties to errors produced by concerto-core.

Use Case

The code parameter allows for errors to be handled programatically easier i.e. an error code of ENTITY_NOT_FOUND, would be a more resilient way to inform consumers than having to parse an error's message.

The status parameter is useful in cases where concerto-core is part of a REST service and the error needs to be produced on a REST API level. One could argue that the REST service is able to assign an HTTP status code based on the error code received alone, however, this would mean having to implement extra code in all of the consumers of concerto-core.

Possible Solution

err.status = '404';
err.code = 'ENTITY_NOT_FOUND';

Context

I'm currently working on a REST API service using cicero-core and it would be nice to get those two parameters out of the box, rather than having to pattern match the error's message which seems fragile.

@jeromesimeon
Copy link
Member

Interesting. Thanks for the new issue @stefanblaginov. Having error codes might be a good idea. I'd love to get more feedback on that from @dselman / @mttrbrts . How important / useful is this compared to other things on our list.

I don't think status 404 makes sense though since this seems more like an http kind of error.

@jeromesimeon
Copy link
Member

@stefanblaginov would you be interested in opening a PR for the error codes part?

@AbhishekJamhoriya
Copy link

@jeromesimeon can i work on this issue .Please assign me this issue.

@dselman dselman added Good First Issue :octocat: Good for newcomers Type: Feature Request 🛍️ New feature or request labels Nov 23, 2021
@pree-T
Copy link

pree-T commented Jan 13, 2022

Hello! I want to work on this issue. Can someone help me as I am new to open source contribution?

@Rishabh044
Copy link

HI!! Can I also work on the following issue, Please assign it to me as well if possible

@amanag25
Copy link

amanag25 commented Jan 24, 2022

@dselman @mttrbrts I think I can solve this issue, it has something to do with the message present in logger.js. Please assign me this issue so that I can get started.

@jeromesimeon
Copy link
Member

Interesting. Thanks for the new issue @stefanblaginov. Having error codes might be a good idea. I'd love to get more feedback on that from @dselman / @mttrbrts . How important / useful is this compared to other things on our list.

I don't think status 404 makes sense though since this seems more like an http kind of error.

@amanag25 @Rishabh044 @pree-T thank you for volunteering to work on this. However, as you can see from the previous comment, I am unclear that we understand the goal or requirements here. This requires some design, and would be best discussed in a specific issue. I will let @stefanblaginov comment on this before assigning work around error codes.

If you would like to help on the project, feel free to look at existing other issues!

@amanag25
Copy link

@jeromesimeon Roger that!😄

@mttrbrts
Copy link
Member

It's polluting to put a protocol specific code (e.g. 404 for HTTP) in this library.

Having standardised error codes is valuable, however.

@jeromesimeon
Copy link
Member

@stefanblaginov any update on this?

@Debarpan08
Copy link

@stefanblaginov @jeromesimeon is this issue still open? I would like to work on this. Could you please assign this to me?

@kailash360
Copy link

kailash360 commented Feb 25, 2024

@stefanblaginov @jeromesimeon

I would like to work on this issue. It's true that hardcoding HTTP codes like 404 will not be a good apporach, so as @mttrbrts suggested, I will create a separate list of codes based on the type of error, and document it for user understanding. I will create a method in the error handler that will take 2 parameters - the status code, and error message, and finally returns the custom object consisting of code and status. I will also define the sets of error codes and messages separately as constants. Let me know if I need to make any modifications in this approach.

@mttrbrts
Copy link
Member

Closed by #773

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 a pull request may close this issue.

10 participants