-
Notifications
You must be signed in to change notification settings - Fork 237
feat(go): use GenkitError
instead of regular error
#2643
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
base: main
Are you sure you want to change the base?
Conversation
GenkitError
instead of regular error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you modify a sample to return errors in various places and verify in the Dev UI that the errors look correct and have a useful stack? Conversely, the stack should not appear when calling the prod server, only the reflection server.
|
||
// Package status defines canonical status codes, names, and related types | ||
// inspired by gRPC status codes. | ||
package core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of exported constants here that aren't qualified, it might make sense to put this in a error
directory and package rather than pollute the core
namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so are you suggesting to create a error package all together ?
As of now it is in sync with the structure that we had in Js and py but go behaves differently so we can think of keeping it as a separate package itself(genkitErr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JS and Python allow importing things piecemeal instead of the entire package like in Go so it's more important for things to be encapsulated well.
Addition of new Genkit error in go:
Resolving : 2476
Checklist (if applicable):