Improve error message when template cannot be found#7245
Improve error message when template cannot be found#7245Coder39179 wants to merge 3 commits intoprojectdiscovery:devfrom
Conversation
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded an exported constant Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/loader/workflow/workflow_loader.go (1)
47-56: Centralize the missing-template guidance string to avoid drift.The same multi-line message is duplicated in both methods (and mirrored in catalog loader). Extracting it into a shared const/helper will keep wording consistent and reduce maintenance cost.
♻️ Proposed refactor
+const missingTemplateGuidance = "Could not find template '%s'.\n" + + "Details: %s\n" + + "Ensure nuclei templates are installed by running:\n" + + " nuclei -update-templates\n" + + "Or specify the templates directory using:\n" + + " -t /path/to/nuclei-templates" ... - gologger.Error().Msgf( - "Could not find template '%s'.\n"+ - "Details: %s\n"+ - "Ensure nuclei templates are installed by running:\n"+ - " nuclei -update-templates\n"+ - "Or specify the templates directory using:\n"+ - " -t /path/to/nuclei-templates", - template, - err, - ) + gologger.Error().Msgf(missingTemplateGuidance, template, err)Also applies to: 74-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/loader/workflow/workflow_loader.go` around lines 47 - 56, Duplicate multi-line missing-template guidance should be centralized: add a single exported const or helper (e.g., MissingTemplateMessage or FormatMissingTemplateMessage(template string, err error)) in the loader package and replace the inline Msgf usage in WorkflowLoader methods (the goroutine that currently logs "Could not find template '%s'..." in workflow_loader.go) and the equivalent code in the catalog loader to call that const/helper instead; ensure the new helper accepts template and err (or returns a formatted string) and update both call sites to use it so wording is consistent across loaders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/loader/workflow/workflow_loader.go`:
- Around line 47-56: Duplicate multi-line missing-template guidance should be
centralized: add a single exported const or helper (e.g., MissingTemplateMessage
or FormatMissingTemplateMessage(template string, err error)) in the loader
package and replace the inline Msgf usage in WorkflowLoader methods (the
goroutine that currently logs "Could not find template '%s'..." in
workflow_loader.go) and the equivalent code in the catalog loader to call that
const/helper instead; ensure the new helper accepts template and err (or returns
a formatted string) and update both call sites to use it so wording is
consistent across loaders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29a2af93-63ab-4d07-a0a0-0448353f97b1
📒 Files selected for processing (2)
pkg/catalog/loader/loader.gopkg/loader/workflow/workflow_loader.go
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/loader/workflow/workflow_loader.go`:
- Line 74: The logging statement is malformed and uses wrong symbols; replace
the garbled call with a proper gologger call that references
loader.MissingTemplateGuidance and the loop variable templatePath, and include
the error via Err(err) instead of the invalid chained calls—e.g., call
gologger.Warning().Err(err).Msgf(loader.MissingTemplateGuidance, templatePath)
(or gologger.Error() if you want error level); ensure you remove the
duplicated/mangled tokens `Warninggologger.Error()` and the illegal
`()().Msg(...)` chaining and use the existing err and templatePath variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0bade59-9caf-40e2-ba6b-7f36d2ad9f71
📒 Files selected for processing (3)
pkg/catalog/loader/loader.gopkg/loader/workflow/workflow_loader.gopkg/protocols/protocols.go
✅ Files skipped from review due to trivial changes (1)
- pkg/protocols/protocols.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/catalog/loader/loader.go
| matched, err := w.options.Parser.LoadTemplate(templatePath, w.tagFilter, nil, w.options.Catalog) | ||
| if err != nil && !matched { | ||
| gologger.Warning().Msg(err.Error()) | ||
| gologger.Warninggologger.Error().Msgf(missingTemplateGuidance, template, err)().Msg(err.Error()) |
There was a problem hiding this comment.
Critical: Malformed logging statement will cause compilation failure.
This line has multiple issues:
gologger.Warninggologger.Error()is garbled syntax (looks like a merge conflict or copy-paste error)missingTemplateGuidanceis undefined; should beloader.MissingTemplateGuidancetemplateis not defined in this scope; the loop variable istemplatePath- The chained
()().Msg(err.Error())is syntactically invalid
This code will not compile and would fail go vet.
🐛 Proposed fix
- gologger.Warninggologger.Error().Msgf(missingTemplateGuidance, template, err)().Msg(err.Error())
+ gologger.Warning().Msgf(loader.MissingTemplateGuidance, templatePath, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| gologger.Warninggologger.Error().Msgf(missingTemplateGuidance, template, err)().Msg(err.Error()) | |
| gologger.Warning().Msgf(loader.MissingTemplateGuidance, templatePath, err) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/loader/workflow/workflow_loader.go` at line 74, The logging statement is
malformed and uses wrong symbols; replace the garbled call with a proper
gologger call that references loader.MissingTemplateGuidance and the loop
variable templatePath, and include the error via Err(err) instead of the invalid
chained calls—e.g., call
gologger.Warning().Err(err).Msgf(loader.MissingTemplateGuidance, templatePath)
(or gologger.Error() if you want error level); ensure you remove the
duplicated/mangled tokens `Warninggologger.Error()` and the illegal
`()().Msg(...)` chaining and use the existing err and templatePath variables.
|
Refactored to centralize the missing-template guidance message and restored an unintentionally modified logging statement. |
dwisiswant0
left a comment
There was a problem hiding this comment.
Before:
Could not find template 'cves/'After:
Could not find template 'cves/'.
Details: could not find template file: no such path found: cves/Ensure nuclei templates are installed by running:
nuclei -update-templates
Or specify the templates directory using:
-t /path/to/nuclei-templates
This does not improve anything nor add value.
The old message is already readable and idiomatic, which tells the user exactly which template failed and what the error was. That is guidance. For the vast majority of cases (missing file, perm issue, wrong path, etc.) this is clear, short, and sufficient.
Your new message is just hand-holding at the cost of code clarity and output noise.
The previous message did not provide clear guidance on how to resolve the issue.
That is simply not true nor a valid reason to add this patch.
We do not accept patches that turn simple, clear errors into verbose tutorials unless there is a demonstrated, frequent user confusion that justifies it.
|
If the goal is to better educate users about missing templates, the right place for that is in the docs repo, not every error path in the code. |
Summary
Improves the error message shown when a template cannot be found.
The previous message did not provide clear guidance on how to resolve the issue.
Changes
nuclei -update-templates-tExample Output
Before:
Could not find template 'cves/'
After:
Could not find template 'cves/'.
Details: could not find template file: no such path found: cves/
Ensure nuclei templates are installed by running:
nuclei -update-templates
Or specify the templates directory using:
-t /path/to/nuclei-templates
Summary by CodeRabbit