fix(loader): replace panic with error handling in template loader when dialers are missing#7054
Open
a638011 wants to merge 3 commits intoprojectdiscovery:devfrom
Open
Conversation
…n dialers are missing
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughLoadTemplates and LoadTemplatesWithTags now return ([]*templates.Template, error). The loader stops panicking when dialers are missing and returns errors instead; callers (lazy auth runner, automaticscan, benchmarks) were updated to handle and propagate these errors. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner (internal/runner)
participant Loader as Template Loader (pkg/catalog/loader)
participant Protocol as ProtocolState/Dialers
Runner->>Loader: LoadTemplates(...)
Loader->>Protocol: GetDialersWithId(execId)
alt dialers nil
Loader-->>Runner: error "dialers with executionId ... not found"
else dialers present
Loader-->>Runner: ([]*templates.Template, nil)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/catalog/loader/loader_bench_test.go`:
- Line 213: The call sites incorrectly unpack two values from
Store.LoadTemplatesOnlyMetadata (which has signature func (store *Store)
LoadTemplatesOnlyMetadata() error); replace occurrences like "_, _ =
store.LoadTemplatesOnlyMetadata()" with a single-value call and handle the error
(for example: "if err := store.LoadTemplatesOnlyMetadata(); err != nil {
t.Fatal(err) }" or assign to err and check), and apply the same change to the
other places that currently try to unpack two return values from
LoadTemplatesOnlyMetadata.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/runner/lazy.gopkg/catalog/loader/loader.gopkg/catalog/loader/loader_bench_test.gopkg/protocols/common/automaticscan/util.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/catalog/loader/loader_bench_test.go`:
- Line 74: The benchmark currently swallows errors by doing "_, _ =
store.LoadTemplates(...)" which can hide loader failures; change each call to
capture the error (e.g., "_, err := store.LoadTemplates(...)") and fail fast
when non-nil (use the benchmark's b.Fatalf or b.Fatal with a clear message and
the err) so the benchmark stops on real load errors; apply this fix for every
occurrence of store.LoadTemplates in the file (the calls shown at lines
referenced in the review).
Don't swallow errors in benchmark loops — use b.Fatalf to surface loader failures instead of silently benchmarking error paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces
panic()calls with proper error propagation in the template loader when dialers are missing or wait group creation fails. Fixes #6674.Changes
pkg/catalog/loader/loader.goLoadTemplatesWithTagssignature to return([]*templates.Template, error)instead of[]*templates.TemplateLoadTemplatessignature to return([]*templates.Template, error)to matchpanic("dialers with executionId ... not found")withreturn nil, fmt.Errorf(...)panic("could not create wait group")withreturn nil, fmt.Errorf(...)dialersvariable assignment (inlined the nil check)Load()to handle the error fromLoadTemplatesby logging itinternal/runner/lazy.goGetLazyAuthFetchCallbackto handle the new error return fromLoadTemplatespkg/protocols/common/automaticscan/util.goLoadTemplatesWithTagscaller to handle the new error returnpkg/catalog/loader/loader_bench_test.go_, _ =for the two-value returnBefore
When dialers were missing or wait group creation failed, the application would
panic(), causing an unrecoverable crash with no meaningful error message to the user.After
Errors are properly propagated up the call chain. Callers can handle the error gracefully (log it, return it, etc.) instead of crashing.
Checklist
LoadTemplatesWithTagsLoadTemplatesgo build ./...)/claim #6674
Summary by CodeRabbit
Bug Fixes
Refactor
Tests