Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions go/core/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ type ReflectionError struct {

// GenkitError is the base error type for Genkit errors.
type GenkitError struct {
Message string `json:"message"` // Exclude from default JSON if embedded elsewhere
Status StatusName `json:"status"`
HTTPCode int `json:"-"` // Exclude from default JSON
Details map[string]any `json:"details"` // Use map for arbitrary details
Source *string `json:"source,omitempty"` // Pointer for optional
Message string `json:"message"` // Exclude from default JSON if embedded elsewhere
Status StatusName `json:"status"`
HTTPCode int `json:"-"` // Exclude from default JSON
Details map[string]any `json:"details"` // Use map for arbitrary details
Source *string `json:"source,omitempty"` // Pointer for optional
originalError error // The wrapped error, if any
}

// UserFacingError is the base error type for user facing errors.
Expand Down Expand Up @@ -78,6 +79,13 @@ func NewError(status StatusName, message string, args ...any) *GenkitError {
Message: fmt.Sprintf(msg, args...),
}

// scan args for the last error to wrap it
for _, arg := range args {
if err, ok := arg.(error); ok {
ge.originalError = err
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The comment says "scan args for the last error to wrap it". The current implementation iterates through all arguments and overwrites ge.originalError if it finds an error. To make this more efficient and to more explicitly match the comment's intent, you could iterate backwards and break after finding the first error (which would be the last one in the args slice).

Suggested change
// scan args for the last error to wrap it
for _, arg := range args {
if err, ok := arg.(error); ok {
ge.originalError = err
}
}
// scan args for the last error to wrap it
for i := len(args) - 1; i >= 0; i-- {
if err, ok := args[i].(error); ok {
ge.originalError = err
break
}
}


errStack := string(debug.Stack())
if errStack != "" {
ge.Details = make(map[string]any)
Expand All @@ -91,14 +99,22 @@ func (e *GenkitError) Error() string {
return e.Message
}

// Unwrap implements the standard error unwrapping interface.
// This allows errors.Is and errors.As to work with GenkitError.
func (e *GenkitError) Unwrap() error {
return e.originalError
}

// ToReflectionError returns a JSON-serializable representation for reflection API responses.
func (e *GenkitError) ToReflectionError() ReflectionError {
errDetails := &ReflectionErrorDetails{}
if stackVal, ok := e.Details["stack"].(string); ok {
errDetails.Stack = &stackVal
}
if traceVal, ok := e.Details["traceId"].(string); ok {
errDetails.TraceID = &traceVal
if e.Details != nil {
if stackVal, ok := e.Details["stack"].(string); ok {
errDetails.Stack = &stackVal
}
if traceVal, ok := e.Details["traceId"].(string); ok {
errDetails.TraceID = &traceVal
}
}
return ReflectionError{
Details: errDetails,
Expand Down
23 changes: 23 additions & 0 deletions go/core/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package core

import (
"errors"
"testing"
)

func TestGenkitErrorUnwrap(t *testing.T) {
original := errors.New("original failure")

// Use INTERNAL instead of StatusInternal
gErr := NewError(INTERNAL, "something happened: %v", original)

// Verify errors.Is works (this is the most important check)
if !errors.Is(gErr, original) {
t.Errorf("expected errors.Is to return true, but got false")
}

// Verify Unwrap works directly
if gErr.Unwrap() != original {
t.Errorf("Unwrap() returned wrong error")
}
}
36 changes: 34 additions & 2 deletions go/core/tracing/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ func Tracer() trace.Tracer {
// such as one that writes to a file.
func WriteTelemetryImmediate(client TelemetryClient) {
e := newTelemetryServerExporter(client)
TracerProvider().RegisterSpanProcessor(sdktrace.NewSimpleSpanProcessor(e))
filtered := &filteringExporter{exporter: e}
TracerProvider().RegisterSpanProcessor(sdktrace.NewSimpleSpanProcessor(filtered))
}

// WriteTelemetryBatch adds a telemetry server to the global tracer provider.
Expand All @@ -146,7 +147,8 @@ func WriteTelemetryImmediate(client TelemetryClient) {
// and perform other cleanup.
func WriteTelemetryBatch(client TelemetryClient) (shutdown func(context.Context) error) {
e := newTelemetryServerExporter(client)
TracerProvider().RegisterSpanProcessor(sdktrace.NewBatchSpanProcessor(e))
filtered := &filteringExporter{exporter: e}
TracerProvider().RegisterSpanProcessor(sdktrace.NewBatchSpanProcessor(filtered))
return TracerProvider().Shutdown
}

Expand Down Expand Up @@ -402,3 +404,33 @@ func SpanPath(ctx context.Context) string {
func SpanTraceInfo(ctx context.Context) TraceInfo {
return spanMetaKey.FromContext(ctx).TraceInfo
}

type filteringExporter struct {
exporter sdktrace.SpanExporter
}

func (e *filteringExporter) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error {
var genkitSpans []sdktrace.ReadOnlySpan
for _, span := range spans {
if isGenkitSpan(span) {
genkitSpans = append(genkitSpans, span)
}
}
if len(genkitSpans) == 0 {
return nil
}
return e.exporter.ExportSpans(ctx, genkitSpans)
}

func (e *filteringExporter) Shutdown(ctx context.Context) error {
return e.exporter.Shutdown(ctx)
}

func isGenkitSpan(span sdktrace.ReadOnlySpan) bool {
for _, attr := range span.Attributes() {
if strings.HasPrefix(string(attr.Key), "genkit") {
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better maintainability and to avoid magic strings, it's best to use the attrPrefix constant that is defined earlier in this file (line 156) instead of the hardcoded string "genkit".

Suggested change
if strings.HasPrefix(string(attr.Key), "genkit") {
if strings.HasPrefix(string(attr.Key), attrPrefix) {

return true
}
}
return false
}