15353 add better script error message (#15441)

* 15353 add better script error message * Simplify _get_script_class() & add docstring --------- Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
netbox-community · May 17, 2024 · 58da5c1 · 58da5c1
1 parent 4b2f26a
commit 58da5c1
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 38 deletions.
diff --git a/netbox/extras/models/scripts.py b/netbox/extras/models/scripts.py
@@ -96,6 +96,7 @@ class ScriptModule(PythonModuleMixin, JobsMixin, ManagedFile):
     Proxy model for script module files.
     """
     objects = ScriptModuleManager()
+    error = None
 
     event_rules = GenericRelation(
         to='extras.EventRule',
@@ -126,6 +127,7 @@ def _get_name(cls):
         try:
             module = self.get_module()
         except Exception as e:
+            self.error = e
             logger.debug(f"Failed to load script: {self.python_name} error: {e}")
             module = None
 

diff --git a/netbox/extras/views.py b/netbox/extras/views.py
@@ -1052,12 +1052,27 @@ def get(self, request):
         })
 
 
-class ScriptView(generic.ObjectView):
+class BaseScriptView(generic.ObjectView):
     queryset = Script.objects.all()
 
+    def _get_script_class(self, script):
+        """
+        Return an instance of the Script's Python class
+        """
+        if script_class := script.python_class:
+            return script_class()
+
+
+class ScriptView(BaseScriptView):
+
     def get(self, request, **kwargs):
         script = self.get_object(**kwargs)
-        script_class = script.python_class()
+        script_class = self._get_script_class(script)
+        if not script_class:
+            return render(request, 'extras/script.html', {
+                'script': script,
+            })
+
         form = script_class.as_form(initial=normalize_querydict(request.GET))
 
         return render(request, 'extras/script.html', {
@@ -1069,11 +1084,16 @@ def get(self, request, **kwargs):
 
     def post(self, request, **kwargs):
         script = self.get_object(**kwargs)
-        script_class = script.python_class()
 
         if not request.user.has_perm('extras.run_script', obj=script):
             return HttpResponseForbidden()
 
+        script_class = self._get_script_class(script)
+        if not script_class:
+            return render(request, 'extras/script.html', {
+                'script': script,
+            })
+
         form = script_class.as_form(request.POST, request.FILES)
 
         # Allow execution only if RQ worker process is running
@@ -1103,21 +1123,22 @@ def post(self, request, **kwargs):
         })
 
 
-class ScriptSourceView(generic.ObjectView):
+class ScriptSourceView(BaseScriptView):
     queryset = Script.objects.all()
 
     def get(self, request, **kwargs):
         script = self.get_object(**kwargs)
+        script_class = self._get_script_class(script)
 
         return render(request, 'extras/script/source.html', {
             'script': script,
-            'script_class': script.python_class(),
+            'script_class': script_class,
             'job_count': script.jobs.count(),
             'tab': 'source',
         })
 
 
-class ScriptJobsView(generic.ObjectView):
+class ScriptJobsView(BaseScriptView):
     queryset = Script.objects.all()
 
     def get(self, request, **kwargs):

diff --git a/netbox/templates/extras/script.html b/netbox/templates/extras/script.html
@@ -14,38 +14,43 @@
           {% trans "You do not have permission to run scripts" %}.
         </div>
       {% endif %}
-      <form action="" method="post" enctype="multipart/form-data" class="object-edit">
-        {% csrf_token %}
-        <div class="field-group my-4">
-          {# Render grouped fields according to declared fieldsets #}
-          {% for group, fields in script_class.get_fieldsets %}
-            {% if fields %}
-              <div class="field-group mb-5">
-                <div class="row">
-                  <h5 class="col-9 offset-3">{{ group }}</h5>
+      {% if form %}
+        <form action="" method="post" enctype="multipart/form-data" class="object-edit">
+          {% csrf_token %}
+          <div class="field-group my-4">
+            {# Render grouped fields according to declared fieldsets #}
+            {% for group, fields in script_class.get_fieldsets %}
+              {% if fields %}
+                <div class="field-group mb-5">
+                  <div class="row">
+                    <h5 class="col-9 offset-3">{{ group }}</h5>
+                  </div>
+                  {% for name in fields %}
+                    {% with field=form|getfield:name %}
+                      {% render_field field %}
+                    {% endwith %}
+                  {% endfor %}
                 </div>
-                {% for name in fields %}
-                  {% with field=form|getfield:name %}
-                    {% render_field field %}
-                  {% endwith %}
-                {% endfor %}
-              </div>
+              {% endif %}
+            {% endfor %}
+          </div>
+          <div class="text-end">
+            <a href="{% url 'extras:script_list' %}" class="btn btn-outline-secondary">{% trans "Cancel" %}</a>
+            {% if not request.user|can_run:script or not script.is_executable %}
+              <button class="btn btn-primary" disabled>
+                <i class="mdi mdi-play"></i> {% trans "Run Script" %}
+              </button>
+            {% else %}
+              <button type="submit" name="_run" class="btn btn-primary">
+                <i class="mdi mdi-play"></i> {% trans "Run Script" %}
+              </button>
             {% endif %}
-          {% endfor %}
-        </div>
-        <div class="text-end">
-          <a href="{% url 'extras:script_list' %}" class="btn btn-outline-secondary">{% trans "Cancel" %}</a>
-          {% if not request.user|can_run:script or not script.is_executable %}
-            <button class="btn btn-primary" disabled>
-              <i class="mdi mdi-play"></i> {% trans "Run Script" %}
-            </button>
-          {% else %}
-            <button type="submit" name="_run" class="btn btn-primary">
-              <i class="mdi mdi-play"></i> {% trans "Run Script" %}
-            </button>
-          {% endif %}
-        </div>
-      </form>
+          </div>
+        </form>
+      {% else %}
+          <p>{% trans "Error loading script" %}.</p>
+          <pre class="block">{{ script.module.error }}</pre>
+      {% endif %}
     </div>
   </div>
 {% endblock content %}
diff --git a/netbox/templates/extras/script/source.html b/netbox/templates/extras/script/source.html
@@ -1,6 +1,14 @@
 {% extends 'extras/script/base.html' %}
+{% load i18n %}
 
 {% block content %}
-  <code class="h6 my-3 d-block">{{ script_class.filename }}</code>
-  <pre class="block">{{ script_class.source }}</pre>
+
+  {% if script_class %}
+    <code class="h6 my-3 d-block">{{ script_class.filename }}</code>
+    <pre class="block">{{ script_class.source }}</pre>
+  {% else %}
+      <p>{% trans "Error loading script" %}.</p>
+      <pre class="block">{{ script.module.error }}</pre>
+  {% endif %}
+
 {% endblock %}