From 7f311311e536f65a69174e8a32d9e410450089d2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Apr 2026 18:48:36 +0000 Subject: [PATCH 1/3] build(deps): bump actions/github-script from 8 to 9 Bumps [actions/github-script](https://github.com/actions/github-script) from 8 to 9. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](https://github.com/actions/github-script/compare/v8...v9) --- updated-dependencies: - dependency-name: actions/github-script dependency-version: '9' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/contract-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/contract-check.yml b/.github/workflows/contract-check.yml index 47d394da..bc38e3c1 100644 --- a/.github/workflows/contract-check.yml +++ b/.github/workflows/contract-check.yml @@ -178,7 +178,7 @@ jobs: - name: Comment PR with contract changes if: steps.contract-diff.outputs.has_changes == 'true' - uses: actions/github-script@v8 + uses: actions/github-script@v9 with: script: | const fs = require('fs'); From 89d59339d11bf6330d16e463e9aee83e6c117ed0 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 6 May 2026 15:55:44 -0400 Subject: [PATCH 2/3] fix: wrap module init panic with static error --- application.go | 2 +- errors.go | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/application.go b/application.go index 6b084486..66f2bca4 100644 --- a/application.go +++ b/application.go @@ -739,7 +739,7 @@ func (app *StdApplication) InitWithApp(appToPass Application) error { defer func() { if r := recover(); r != nil { mu.Lock() - errs = append(errs, fmt.Errorf("panic initializing module %s: %v", name, r)) + errs = append(errs, fmt.Errorf("%w %s: %v", ErrModuleInitializationPanic, name, r)) mu.Unlock() } }() diff --git a/errors.go b/errors.go index 0b2c2528..2d9fe570 100644 --- a/errors.go +++ b/errors.go @@ -87,12 +87,13 @@ var ( ErrTenantIsolationViolation = errors.New("tenant isolation violation") // Reload errors - ErrReloadCircuitBreakerOpen = errors.New("reload circuit breaker is open; backing off") - ErrReloadChannelFull = errors.New("reload request channel is full") - ErrReloadInProgress = errors.New("reload already in progress") - ErrReloadStopped = errors.New("reload orchestrator is stopped") - ErrReloadTimeout = errors.New("reload timed out waiting for module") - ErrDynamicReloadNotEnabled = errors.New("dynamic reload not enabled") + ErrReloadCircuitBreakerOpen = errors.New("reload circuit breaker is open; backing off") + ErrReloadChannelFull = errors.New("reload request channel is full") + ErrReloadInProgress = errors.New("reload already in progress") + ErrReloadStopped = errors.New("reload orchestrator is stopped") + ErrReloadTimeout = errors.New("reload timed out waiting for module") + ErrDynamicReloadNotEnabled = errors.New("dynamic reload not enabled") + ErrModuleInitializationPanic = errors.New("panic initializing module") // Observer/Event emission errors ErrNoSubjectForEventEmission = errors.New("no subject available for event emission") From 2f964b8aab9aca3bd404f8bda286743b1403068b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 19 May 2026 23:01:49 +0000 Subject: [PATCH 3/3] fix: resolve linter errors and add test coverage for panic error wrapping - Add ErrReloadPanic and ErrHealthCheckPanic static errors to errors.go - Fix contract_verifier.go to wrap panics with static errors (fixes err113) - Add Err field to ContractViolation for structured error access - Add specific violations for Reload/HealthCheck panics with dedicated rules - Protect checkReloadRespectsCancel from unhandled Reload panics - Replace reflect.Ptr with reflect.Pointer across all feeder files (govet) - Fix gofmt alignment in feeder struct fields - Add test for parallel init panic wrapping ErrModuleInitializationPanic - Add test for reload panic wrapping ErrReloadPanic - Add test for health check panic wrapping ErrHealthCheckPanic Agent-Logs-Url: https://github.com/GoCodeAlone/modular/sessions/a35133a8-035d-423e-849c-7ff65d31641d Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- chimux/module.go | 2 +- contract_verifier.go | 33 ++++++++++-- contract_verifier_test.go | 51 +++++++++++++++++++ errors.go | 2 + feeders/affixed_env.go | 6 +-- feeders/base_config.go | 2 +- feeders/dot_env.go | 10 ++-- feeders/env.go | 94 +++++++++++++++++------------------ feeders/instance_aware_env.go | 50 +++++++++---------- feeders/json.go | 12 ++--- feeders/toml.go | 12 ++--- feeders/yaml.go | 56 ++++++++++----------- parallel_init_test.go | 37 ++++++++++++++ service.go | 2 +- 14 files changed, 242 insertions(+), 127 deletions(-) diff --git a/chimux/module.go b/chimux/module.go index 61616947..af90d9bc 100644 --- a/chimux/module.go +++ b/chimux/module.go @@ -378,7 +378,7 @@ func (m *ChiMuxModule) setupMiddleware(app modular.Application) error { middlewareProviderType := reflect.TypeOf((*MiddlewareProvider)(nil)).Elem() if serviceType.Implements(middlewareProviderType) || - (serviceType.Kind() == reflect.Ptr && serviceType.Elem().Implements(middlewareProviderType)) { + (serviceType.Kind() == reflect.Pointer && serviceType.Elem().Implements(middlewareProviderType)) { if provider, ok := service.(MiddlewareProvider); ok { middlewareProviders = append(middlewareProviders, provider) m.logger.Debug("Found middleware provider", "name", name) diff --git a/contract_verifier.go b/contract_verifier.go index b9bca84a..612c0cfd 100644 --- a/contract_verifier.go +++ b/contract_verifier.go @@ -2,6 +2,7 @@ package modular import ( "context" + "errors" "fmt" "sync" "time" @@ -13,6 +14,7 @@ type ContractViolation struct { Rule string // e.g., "must-return-positive-timeout" Description string Severity string // "error" or "warning" + Err error // underlying error, if applicable } // ContractVerifier verifies that implementations of Reloadable and HealthProvider @@ -60,11 +62,16 @@ func (v *StandardContractVerifier) VerifyReloadContract(module Reloadable) []Con // 3. Reload with empty changes should be idempotent. if err := v.checkReloadIdempotent(module); err != nil { + rule := "empty-reload-must-be-idempotent" + if errors.Is(err, ErrReloadPanic) { + rule = "reload-must-not-panic" + } violations = append(violations, ContractViolation{ Contract: "reload", - Rule: "empty-reload-must-be-idempotent", + Rule: rule, Description: fmt.Sprintf("Reload() with empty changes failed: %v", err), Severity: "warning", + Err: err, }) } @@ -130,7 +137,7 @@ func (v *StandardContractVerifier) runReloadWithGuard(module Reloadable, label s go func() { defer func() { if r := recover(); r != nil { - ch <- result{err: fmt.Errorf("Reload panicked: %v", r)} + ch <- result{err: fmt.Errorf("%w: %v", ErrReloadPanic, r)} } }() ch <- result{err: module.Reload(ctx, nil)} @@ -149,10 +156,17 @@ func (v *StandardContractVerifier) runReloadWithGuard(module Reloadable, label s // checkReloadRespectsCancel calls Reload with an already-cancelled context and // returns true if Reload returned an error (i.e., it respected the cancellation). -func (v *StandardContractVerifier) checkReloadRespectsCancel(module Reloadable) bool { +// A panic from Reload is treated as "respected cancellation" (error path). +func (v *StandardContractVerifier) checkReloadRespectsCancel(module Reloadable) (respected bool) { ctx, cancel := context.WithCancel(context.Background()) cancel() // cancel immediately + defer func() { + if r := recover(); r != nil { + // A panic counts as a non-nil error path. + respected = true + } + }() err := module.Reload(ctx, nil) return err != nil } @@ -177,7 +191,7 @@ func (v *StandardContractVerifier) VerifyHealthContract(provider HealthProvider) go func() { defer func() { if r := recover(); r != nil { - ch <- result{err: fmt.Errorf("HealthCheck panicked: %v", r)} + ch <- result{err: fmt.Errorf("%w: %v", ErrHealthCheckPanic, r)} } }() reports, err := provider.HealthCheck(ctx) @@ -195,6 +209,17 @@ func (v *StandardContractVerifier) VerifyHealthContract(provider HealthProvider) // Can't check fields if we timed out. return violations case res := <-ch: + if errors.Is(res.err, ErrHealthCheckPanic) { + violations = append(violations, ContractViolation{ + Contract: "health", + Rule: "health-check-must-not-panic", + Description: fmt.Sprintf("HealthCheck() panicked: %v", res.err), + Severity: "error", + Err: res.err, + }) + // Skip further checks since the provider panics. + return violations + } if res.err == nil { for _, report := range res.reports { if report.Module == "" { diff --git a/contract_verifier_test.go b/contract_verifier_test.go index d5520c3d..27d9ab94 100644 --- a/contract_verifier_test.go +++ b/contract_verifier_test.go @@ -2,6 +2,7 @@ package modular import ( "context" + "errors" "testing" "time" ) @@ -30,6 +31,13 @@ type panickyReloadable struct{ wellBehavedReloadable } func (p *panickyReloadable) CanReload() bool { panic("boom") } +// reloadPanickyReloadable panics when Reload is called. +type reloadPanickyReloadable struct{ wellBehavedReloadable } + +func (r *reloadPanickyReloadable) Reload(_ context.Context, _ []ConfigChange) error { + panic("reload boom") +} + // --- Mock HealthProviders for contract tests --- // wellBehavedHealthProvider returns a proper report and respects cancellation. @@ -81,6 +89,13 @@ func (c *cancelIgnoringHealthProvider) HealthCheck(_ context.Context) ([]HealthR }, nil } +// panickyHealthProvider panics when HealthCheck is called. +type panickyHealthProvider struct{} + +func (p *panickyHealthProvider) HealthCheck(_ context.Context) ([]HealthReport, error) { + panic("health check boom") +} + // --- Tests --- func TestContractVerifier_ReloadWellBehaved(t *testing.T) { @@ -162,3 +177,39 @@ func TestContractVerifier_HealthIgnoresCancellation(t *testing.T) { t.Fatalf("expected violation for ignoring cancellation, got: %+v", violations) } } + +// TestContractVerifier_ReloadPanicWrapsErrReloadPanic verifies that a panic inside +// Reload is captured and the resulting violation error wraps ErrReloadPanic. +func TestContractVerifier_ReloadPanicWrapsErrReloadPanic(t *testing.T) { + verifier := NewStandardContractVerifier() + violations := verifier.VerifyReloadContract(&reloadPanickyReloadable{}) + + found := false + for _, v := range violations { + if v.Rule == "reload-must-not-panic" && errors.Is(v.Err, ErrReloadPanic) { + found = true + break + } + } + if !found { + t.Fatalf("expected violation with ErrReloadPanic, got: %+v", violations) + } +} + +// TestContractVerifier_HealthPanicWrapsErrHealthCheckPanic verifies that a panic inside +// HealthCheck is captured and the resulting violation error wraps ErrHealthCheckPanic. +func TestContractVerifier_HealthPanicWrapsErrHealthCheckPanic(t *testing.T) { + verifier := NewStandardContractVerifier() + violations := verifier.VerifyHealthContract(&panickyHealthProvider{}) + + found := false + for _, v := range violations { + if v.Rule == "health-check-must-not-panic" && errors.Is(v.Err, ErrHealthCheckPanic) { + found = true + break + } + } + if !found { + t.Fatalf("expected violation with ErrHealthCheckPanic, got: %+v", violations) + } +} diff --git a/errors.go b/errors.go index 2d9fe570..fae30af4 100644 --- a/errors.go +++ b/errors.go @@ -94,6 +94,8 @@ var ( ErrReloadTimeout = errors.New("reload timed out waiting for module") ErrDynamicReloadNotEnabled = errors.New("dynamic reload not enabled") ErrModuleInitializationPanic = errors.New("panic initializing module") + ErrReloadPanic = errors.New("reload panicked") + ErrHealthCheckPanic = errors.New("health check panicked") // Observer/Event emission errors ErrNoSubjectForEventEmission = errors.New("no subject available for event emission") diff --git a/feeders/affixed_env.go b/feeders/affixed_env.go index 07d4f862..79ce56a7 100644 --- a/feeders/affixed_env.go +++ b/feeders/affixed_env.go @@ -29,8 +29,8 @@ type AffixedEnvFeeder struct { logger interface { Debug(msg string, args ...any) } - ft FieldTrackerHolder - priority int + ft FieldTrackerHolder + priority int } // NewAffixedEnvFeeder creates a new AffixedEnvFeeder with the specified prefix and suffix @@ -79,7 +79,7 @@ func (f *AffixedEnvFeeder) Feed(structure interface{}) error { inputType := reflect.TypeOf(structure) if inputType != nil { - if inputType.Kind() == reflect.Ptr { + if inputType.Kind() == reflect.Pointer { if inputType.Elem().Kind() == reflect.Struct { return f.fillStruct(reflect.ValueOf(structure).Elem(), f.Prefix, f.Suffix) } diff --git a/feeders/base_config.go b/feeders/base_config.go index e4cccd83..3db85645 100644 --- a/feeders/base_config.go +++ b/feeders/base_config.go @@ -18,7 +18,7 @@ type BaseConfigFeeder struct { Environment string // Environment name (e.g., "prod", "staging", "dev") verboseDebug bool logger interface{ Debug(msg string, args ...any) } - ft FieldTrackerHolder + ft FieldTrackerHolder } // NewBaseConfigFeeder creates a new base configuration feeder diff --git a/feeders/dot_env.go b/feeders/dot_env.go index 3d070972..10591f37 100644 --- a/feeders/dot_env.go +++ b/feeders/dot_env.go @@ -16,9 +16,9 @@ type DotEnvFeeder struct { logger interface { Debug(msg string, args ...any) } - ft FieldTrackerHolder - envVars map[string]string // in-memory storage of parsed .env variables - priority int + ft FieldTrackerHolder + envVars map[string]string // in-memory storage of parsed .env variables + priority int } // NewDotEnvFeeder creates a new DotEnvFeeder that reads from the specified .env file @@ -170,7 +170,7 @@ func (f *DotEnvFeeder) parseEnvLine(line string, lineNum int) error { // populateStructFromCatalog populates struct fields from the global environment catalog func (f *DotEnvFeeder) populateStructFromCatalog(structure interface{}, prefix string) error { structValue := reflect.ValueOf(structure) - if structValue.Kind() != reflect.Ptr || structValue.Elem().Kind() != reflect.Struct { + if structValue.Kind() != reflect.Pointer || structValue.Elem().Kind() != reflect.Struct { return wrapDotEnvStructureError(structure) } @@ -300,7 +300,7 @@ func (f *DotEnvFeeder) convertStringToType(value string, targetType reflect.Type return boolVal, nil case reflect.Invalid, reflect.Uintptr, reflect.Complex64, reflect.Complex128, reflect.Array, reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, - reflect.Ptr, reflect.Slice, reflect.Struct, reflect.UnsafePointer: + reflect.Pointer, reflect.Slice, reflect.Struct, reflect.UnsafePointer: return nil, wrapDotEnvUnsupportedTypeError(targetType.Kind().String()) default: return nil, wrapDotEnvUnsupportedTypeError(targetType.Kind().String()) diff --git a/feeders/env.go b/feeders/env.go index 1891e655..a5ff3958 100644 --- a/feeders/env.go +++ b/feeders/env.go @@ -12,8 +12,8 @@ type EnvFeeder struct { logger interface { Debug(msg string, args ...any) } - ft FieldTrackerHolder - priority int + ft FieldTrackerHolder + priority int } // NewEnvFeeder creates a new EnvFeeder that reads from environment variables @@ -75,7 +75,7 @@ func (f *EnvFeeder) FeedWithModuleContext(structure interface{}, moduleName stri return ErrEnvInvalidStructure } - if inputType.Kind() != reflect.Ptr { + if inputType.Kind() != reflect.Pointer { if f.verboseDebug && f.logger != nil { f.logger.Debug("EnvFeeder: Structure is not a pointer", "kind", inputType.Kind()) } @@ -231,17 +231,17 @@ func (f *EnvFeeder) setFieldFromEnvWithModule(field reflect.Value, envTag, prefi // Record field population if tracker is available f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.EnvFeeder", - SourceType: "env", - SourceKey: foundKey, - Value: field.Interface(), - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: foundKey, - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.EnvFeeder", + SourceType: "env", + SourceKey: foundKey, + Value: field.Interface(), + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: foundKey, + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("EnvFeeder: Successfully set field value", "fieldName", fieldName, "foundKey", foundKey, "envValue", envValue, "fieldPath", fieldPath) @@ -249,17 +249,17 @@ func (f *EnvFeeder) setFieldFromEnvWithModule(field reflect.Value, envTag, prefi } else { // Record that we searched but didn't find f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.EnvFeeder", - SourceType: "env", - SourceKey: "", - Value: nil, - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: "", - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.EnvFeeder", + SourceType: "env", + SourceKey: "", + Value: nil, + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: "", + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("EnvFeeder: Environment variable not found or empty", "fieldName", fieldName, "searchKeys", searchKeys, "fieldPath", fieldPath) @@ -346,17 +346,17 @@ func (f *EnvFeeder) setPointerFieldFromEnvWithModule(field reflect.Value, envTag // Record field population if tracker is available f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.EnvFeeder", - SourceType: "env", - SourceKey: foundKey, - Value: field.Interface(), - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: foundKey, - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.EnvFeeder", + SourceType: "env", + SourceKey: foundKey, + Value: field.Interface(), + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: foundKey, + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("EnvFeeder: Successfully set pointer field", "fieldName", fieldName, "foundKey", foundKey, "fieldPath", fieldPath) @@ -364,17 +364,17 @@ func (f *EnvFeeder) setPointerFieldFromEnvWithModule(field reflect.Value, envTag } else { // Record that we searched but didn't find f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.EnvFeeder", - SourceType: "env", - SourceKey: "", - Value: nil, - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: "", - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.EnvFeeder", + SourceType: "env", + SourceKey: "", + Value: nil, + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: "", + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("EnvFeeder: Environment variable not found or empty for pointer field", "fieldName", fieldName, "searchKeys", searchKeys, "fieldPath", fieldPath) diff --git a/feeders/instance_aware_env.go b/feeders/instance_aware_env.go index 0926122a..51fc72e1 100644 --- a/feeders/instance_aware_env.go +++ b/feeders/instance_aware_env.go @@ -73,7 +73,7 @@ func (f *InstanceAwareEnvFeeder) Feed(structure interface{}) error { return ErrEnvInvalidStructure } - if inputType.Kind() != reflect.Ptr { + if inputType.Kind() != reflect.Pointer { if f.verboseDebug && f.logger != nil { f.logger.Debug("InstanceAwareEnvFeeder: Structure is not a pointer", "kind", inputType.Kind()) } @@ -119,7 +119,7 @@ func (f *InstanceAwareEnvFeeder) FeedKey(instanceKey string, structure interface return ErrEnvInvalidStructure } - if inputType.Kind() != reflect.Ptr { + if inputType.Kind() != reflect.Pointer { if f.verboseDebug && f.logger != nil { f.logger.Debug("InstanceAwareEnvFeeder: Structure is not a pointer", "instanceKey", instanceKey, "kind", inputType.Kind()) } @@ -188,7 +188,7 @@ func (f *InstanceAwareEnvFeeder) FeedInstances(instances interface{}) error { var needsMapUpdate bool // Handle both pointer and non-pointer map values - if instance.Kind() == reflect.Ptr { + if instance.Kind() == reflect.Pointer { // Map values are already pointers - use them directly instancePtr = instance.Interface() needsMapUpdate = false // No need to update map since we're modifying the original pointer @@ -344,17 +344,17 @@ func (f *InstanceAwareEnvFeeder) setFieldFromEnvWithPrefix(field reflect.Value, // Record field population if tracker is available f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.InstanceAwareEnvFeeder", - SourceType: "env", - SourceKey: envName, - Value: field.Interface(), - InstanceKey: instanceKey, - SearchKeys: searchKeys, - FoundKey: envName, - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.InstanceAwareEnvFeeder", + SourceType: "env", + SourceKey: envName, + Value: field.Interface(), + InstanceKey: instanceKey, + SearchKeys: searchKeys, + FoundKey: envName, + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("InstanceAwareEnvFeeder: Successfully set field value", "envName", envName, "envValue", envValue, "fieldPath", fieldPath, "instanceKey", instanceKey) @@ -362,17 +362,17 @@ func (f *InstanceAwareEnvFeeder) setFieldFromEnvWithPrefix(field reflect.Value, } else { // Record that we searched but didn't find f.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.InstanceAwareEnvFeeder", - SourceType: "env", - SourceKey: "", - Value: nil, - InstanceKey: instanceKey, - SearchKeys: searchKeys, - FoundKey: "", - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.InstanceAwareEnvFeeder", + SourceType: "env", + SourceKey: "", + Value: nil, + InstanceKey: instanceKey, + SearchKeys: searchKeys, + FoundKey: "", + }) if f.verboseDebug && f.logger != nil { f.logger.Debug("InstanceAwareEnvFeeder: Environment variable not found or empty", "envName", envName, "fieldPath", fieldPath, "instanceKey", instanceKey) diff --git a/feeders/json.go b/feeders/json.go index 814704ca..9efc5ec5 100644 --- a/feeders/json.go +++ b/feeders/json.go @@ -57,8 +57,8 @@ type JSONFeeder struct { logger interface { Debug(msg string, args ...any) } - ft FieldTrackerHolder - priority int + ft FieldTrackerHolder + priority int } // NewJSONFeeder creates a new JSONFeeder that reads from the specified JSON file @@ -153,7 +153,7 @@ func (j *JSONFeeder) feedWithTracking(structure interface{}) error { // Check if we're dealing with a struct pointer structValue := reflect.ValueOf(structure) - if structValue.Kind() != reflect.Ptr || structValue.Elem().Kind() != reflect.Struct { + if structValue.Kind() != reflect.Pointer || structValue.Elem().Kind() != reflect.Struct { // Not a struct pointer, fall back to standard JSON unmarshaling if j.verboseDebug && j.logger != nil { j.logger.Debug("JSONFeeder: Not a struct pointer, using standard JSON unmarshaling", "structureType", reflect.TypeOf(structure)) @@ -220,7 +220,7 @@ func (j *JSONFeeder) processField(field reflect.Value, fieldType reflect.StructF fieldKind := field.Kind() switch fieldKind { - case reflect.Ptr: + case reflect.Pointer: // Handle pointer types return j.setPointerFromJSON(field, value, fieldPath) @@ -454,7 +454,7 @@ func (j *JSONFeeder) setSliceFromJSON(field reflect.Value, value interface{}, fi } else { return wrapJSONSliceElementError(item, elemType.String(), fieldPath, i) } - case reflect.Ptr: + case reflect.Pointer: // Handle slice of pointers if item == nil { // Set nil pointer @@ -553,7 +553,7 @@ func (j *JSONFeeder) setMapFromJSON(field reflect.Value, jsonData map[string]int } } } - case reflect.Ptr: + case reflect.Pointer: // Map of pointers to structs, like map[string]*DBConnection elemType := valueType.Elem() if elemType.Kind() == reflect.Struct { diff --git a/feeders/toml.go b/feeders/toml.go index d3ac64b9..2b411c9e 100644 --- a/feeders/toml.go +++ b/feeders/toml.go @@ -17,8 +17,8 @@ type TomlFeeder struct { logger interface { Debug(msg string, args ...any) } - ft FieldTrackerHolder - priority int + ft FieldTrackerHolder + priority int } // NewTomlFeeder creates a new TomlFeeder that reads from the specified TOML file @@ -113,7 +113,7 @@ func (t *TomlFeeder) feedWithTracking(structure interface{}) error { // Check if we're dealing with a struct pointer structValue := reflect.ValueOf(structure) - if structValue.Kind() != reflect.Ptr || structValue.Elem().Kind() != reflect.Struct { + if structValue.Kind() != reflect.Pointer || structValue.Elem().Kind() != reflect.Struct { // Not a struct pointer, fall back to standard TOML unmarshaling if t.verboseDebug && t.logger != nil { t.logger.Debug("TomlFeeder: Not a struct pointer, using standard TOML unmarshaling", "structureType", reflect.TypeOf(structure)) @@ -180,7 +180,7 @@ func (t *TomlFeeder) processField(field reflect.Value, fieldType reflect.StructF fieldKind := field.Kind() switch fieldKind { - case reflect.Ptr: + case reflect.Pointer: // Handle pointer types return t.setPointerFromTOML(field, value, fieldPath) @@ -428,7 +428,7 @@ func (t *TomlFeeder) setSliceFromTOML(field reflect.Value, value interface{}, fi } else { return wrapTomlSliceElementError(item, elemType.String(), fieldPath, i) } - case reflect.Ptr: + case reflect.Pointer: // Handle slice of pointers if item == nil { // Set nil pointer @@ -524,7 +524,7 @@ func (t *TomlFeeder) setMapFromTOML(field reflect.Value, tomlData map[string]int } } } - case reflect.Ptr: + case reflect.Pointer: // Map of pointers to structs, like map[string]*DBConnection elemType := valueType.Elem() if elemType.Kind() == reflect.Struct { diff --git a/feeders/yaml.go b/feeders/yaml.go index a6353437..64a03b08 100644 --- a/feeders/yaml.go +++ b/feeders/yaml.go @@ -57,7 +57,7 @@ type YamlFeeder struct { Path string verboseDebug bool debugFn func(string) - ft FieldTrackerHolder + ft FieldTrackerHolder priority int } @@ -193,7 +193,7 @@ func (y *YamlFeeder) feedWithTracking(structure interface{}) error { // Check if we're dealing with a struct pointer structValue := reflect.ValueOf(structure) - if structValue.Kind() != reflect.Ptr || structValue.Elem().Kind() != reflect.Struct { + if structValue.Kind() != reflect.Pointer || structValue.Elem().Kind() != reflect.Struct { // Not a struct pointer, fall back to standard YAML unmarshaling y.debugLog("YamlFeeder: Not a struct pointer, using standard YAML unmarshaling", "structureType", reflect.TypeOf(structure)) if err := yaml.Unmarshal(content, structure); err != nil { @@ -245,7 +245,7 @@ func (y *YamlFeeder) processField(field reflect.Value, fieldType *reflect.Struct fieldName, hasYAMLTag := getFieldNameFromTag(fieldType) switch field.Kind() { - case reflect.Ptr: + case reflect.Pointer: // Handle pointer types if hasYAMLTag { return y.setPointerFromYAML(field, fieldName, data, fieldType.Name, fieldPath) @@ -444,7 +444,7 @@ func (y *YamlFeeder) setSliceFromYAML(field reflect.Value, yamlTag string, data } else { return wrapYamlExpectedMapForSliceError(fieldPath, i, item) } - case reflect.Ptr: + case reflect.Pointer: // Handle slice of pointers if item == nil { // Set nil pointer @@ -577,33 +577,33 @@ func (y *YamlFeeder) setFieldFromYaml(field reflect.Value, yamlTag string, data // Record field population if tracker is available y.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.YamlFeeder", - SourceType: "yaml", - SourceKey: foundKey, - Value: field.Interface(), - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: foundKey, - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.YamlFeeder", + SourceType: "yaml", + SourceKey: foundKey, + Value: field.Interface(), + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: foundKey, + }) y.debugLog("YamlFeeder: Successfully set field value", "fieldName", fieldName, "yamlKey", yamlTag, "value", foundValue, "fieldPath", fieldPath) } else { // Record that we searched but didn't find y.ft.Record(FieldPopulation{ - FieldPath: fieldPath, - FieldName: fieldName, - FieldType: field.Type().String(), - FeederType: "*feeders.YamlFeeder", - SourceType: "yaml", - SourceKey: "", - Value: nil, - InstanceKey: "", - SearchKeys: searchKeys, - FoundKey: "", - }) + FieldPath: fieldPath, + FieldName: fieldName, + FieldType: field.Type().String(), + FeederType: "*feeders.YamlFeeder", + SourceType: "yaml", + SourceKey: "", + Value: nil, + InstanceKey: "", + SearchKeys: searchKeys, + FoundKey: "", + }) y.debugLog("YamlFeeder: YAML value not found", "fieldName", fieldName, "yamlKey", yamlTag, "fieldPath", fieldPath) } @@ -647,7 +647,7 @@ func (y *YamlFeeder) setMapFromYaml(field reflect.Value, yamlData map[string]int y.debugLog("YamlFeeder: Map entry is not a map", "key", key, "valueType", reflect.TypeOf(value)) } } - case reflect.Ptr: + case reflect.Pointer: // Map of pointers to structs, like map[string]*DBConnection elemType := valueType.Elem() if elemType.Kind() == reflect.Struct { @@ -815,7 +815,7 @@ func (y *YamlFeeder) setFieldValue(field reflect.Value, value interface{}) error } case reflect.Invalid, reflect.Uintptr, reflect.Complex64, reflect.Complex128, reflect.Array, reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, - reflect.Ptr, reflect.Slice, reflect.Struct, reflect.UnsafePointer: + reflect.Pointer, reflect.Slice, reflect.Struct, reflect.UnsafePointer: return wrapYamlUnsupportedFieldTypeError(field.Type().String()) default: return wrapYamlUnsupportedFieldTypeError(field.Type().String()) diff --git a/parallel_init_test.go b/parallel_init_test.go index 35fce24a..8cac021e 100644 --- a/parallel_init_test.go +++ b/parallel_init_test.go @@ -1,6 +1,7 @@ package modular import ( + "errors" "sync" "sync/atomic" "testing" @@ -110,3 +111,39 @@ func (m *simpleOrderModule) Init(app Application) error { m.mu.Unlock() return nil } + +// panicOnInitModule panics during Init to exercise the panic-recovery path. +type panicOnInitModule struct { + name string +} + +func (m *panicOnInitModule) Name() string { return m.name } +func (m *panicOnInitModule) Dependencies() []string { return nil } +func (m *panicOnInitModule) Init(_ Application) error { + panic("intentional panic during init") +} + +// TestWithParallelInit_PanicWrapsErrModuleInitializationPanic verifies that a panic +// during parallel module initialisation is wrapped with ErrModuleInitializationPanic. +func TestWithParallelInit_PanicWrapsErrModuleInitializationPanic(t *testing.T) { + // Register two panic-on-init modules at the same depth so they run concurrently. + modA := &panicOnInitModule{name: "panic-a"} + modB := &panicOnInitModule{name: "panic-b"} + + app, err := NewApplication( + WithLogger(nopLogger{}), + WithModules(modA, modB), + WithParallelInit(), + ) + if err != nil { + t.Fatalf("NewApplication: %v", err) + } + + initErr := app.Init() + if initErr == nil { + t.Fatal("expected Init to return an error after panic, got nil") + } + if !errors.Is(initErr, ErrModuleInitializationPanic) { + t.Errorf("expected error to wrap ErrModuleInitializationPanic, got: %v", initErr) + } +} diff --git a/service.go b/service.go index d7d2d475..990d6930 100644 --- a/service.go +++ b/service.go @@ -254,7 +254,7 @@ func (r *EnhancedServiceRegistry) generateUniqueName(originalName, moduleName st // Still conflicts - try with module type name if moduleType != nil { var typeName string - if moduleType.Kind() == reflect.Ptr { + if moduleType.Kind() == reflect.Pointer { typeName = moduleType.Elem().Name() } if typeName == "" {