From 9254d19b741a2c323376b46ec104b8c031475e85 Mon Sep 17 00:00:00 2001 From: jasquat Date: Wed, 19 Apr 2023 15:20:19 -0400 Subject: [PATCH] display event errors in the frontend using errorDisplay w/ burnettk --- .../{c95031498e62_.py => e0510795b643_.py} | 10 +- spiffworkflow-backend/poetry.lock | 6 +- spiffworkflow-backend/pyproject.toml | 2 +- .../models/process_instance_error_detail.py | 8 +- .../routes/process_instances_controller.py | 2 + .../services/task_service.py | 19 +++- .../services/workflow_execution_service.py | 2 + .../src/components/ErrorDisplay.tsx | 93 ++++++++++--------- spiffworkflow-frontend/src/interfaces.ts | 7 +- .../src/routes/ProcessInstanceLogList.tsx | 17 +++- 10 files changed, 112 insertions(+), 54 deletions(-) rename spiffworkflow-backend/migrations/versions/{c95031498e62_.py => e0510795b643_.py} (99%) diff --git a/spiffworkflow-backend/migrations/versions/c95031498e62_.py b/spiffworkflow-backend/migrations/versions/e0510795b643_.py similarity index 99% rename from spiffworkflow-backend/migrations/versions/c95031498e62_.py rename to spiffworkflow-backend/migrations/versions/e0510795b643_.py index c375745c..d613a74a 100644 --- a/spiffworkflow-backend/migrations/versions/c95031498e62_.py +++ b/spiffworkflow-backend/migrations/versions/e0510795b643_.py @@ -1,8 +1,8 @@ """empty message -Revision ID: c95031498e62 +Revision ID: e0510795b643 Revises: -Create Date: 2023-04-19 10:35:25.813002 +Create Date: 2023-04-19 14:36:11.004444 """ from alembic import op @@ -10,7 +10,7 @@ import sqlalchemy as sa from sqlalchemy.dialects import mysql # revision identifiers, used by Alembic. -revision = 'c95031498e62' +revision = 'e0510795b643' down_revision = None branch_labels = None depends_on = None @@ -468,6 +468,10 @@ def upgrade(): sa.Column('process_instance_event_id', sa.Integer(), nullable=False), sa.Column('message', sa.String(length=1024), nullable=False), sa.Column('stacktrace', sa.Text(), nullable=False), + sa.Column('task_line_number', sa.Integer(), nullable=True), + sa.Column('task_offset', sa.Integer(), nullable=True), + sa.Column('task_line_contents', sa.String(length=255), nullable=True), + sa.Column('task_trace', sa.JSON(), nullable=True), sa.ForeignKeyConstraint(['process_instance_event_id'], ['process_instance_event.id'], ), sa.PrimaryKeyConstraint('id') ) diff --git a/spiffworkflow-backend/poetry.lock b/spiffworkflow-backend/poetry.lock index d26d0042..5ba2205a 100644 --- a/spiffworkflow-backend/poetry.lock +++ b/spiffworkflow-backend/poetry.lock @@ -1923,8 +1923,8 @@ lxml = "*" [package.source] type = "git" url = "https://github.com/sartography/SpiffWorkflow" -reference = "main" -resolved_reference = "7211e67ee0dfecbabaeb7cec8f0e0373bd7cdc10" +reference = "feature/new-task-states" +resolved_reference = "5a0fd2774fde20dd65dbb49ae67f22cad53df528" [[package]] name = "sqlalchemy" @@ -2307,7 +2307,7 @@ testing = ["big-O", "flake8 (<5)", "jaraco.functools", "jaraco.itertools", "more [metadata] lock-version = "1.1" python-versions = ">=3.9,<3.12" -content-hash = "994c36ab39238500b4fd05bc1ccdd2d729dd5f66749ab77b1028371147bdf753" +content-hash = "4ae3c31115f378193f33eb9b27d376fcf0f7c20fba54ed5c921f37a4778b8d09" [metadata.files] alabaster = [ diff --git a/spiffworkflow-backend/pyproject.toml b/spiffworkflow-backend/pyproject.toml index d4288f5b..bfa0ad07 100644 --- a/spiffworkflow-backend/pyproject.toml +++ b/spiffworkflow-backend/pyproject.toml @@ -27,7 +27,7 @@ flask-marshmallow = "*" flask-migrate = "*" flask-restful = "*" werkzeug = "*" -SpiffWorkflow = {git = "https://github.com/sartography/SpiffWorkflow", rev = "main"} +SpiffWorkflow = {git = "https://github.com/sartography/SpiffWorkflow", rev = "feature/new-task-states"} # SpiffWorkflow = {git = "https://github.com/sartography/SpiffWorkflow", rev = "6cad2981712bb61eca23af1adfafce02d3277cb9"} # SpiffWorkflow = {develop = true, path = "../../SpiffWorkflow" } sentry-sdk = "^1.10" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_error_detail.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_error_detail.py index d1a38c49..663ab6ce 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_error_detail.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/process_instance_error_detail.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +from typing import Optional from sqlalchemy.orm import relationship from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel from sqlalchemy import ForeignKey @@ -11,9 +12,14 @@ class ProcessInstanceErrorDetailModel(SpiffworkflowBaseDBModel): id: int = db.Column(db.Integer, primary_key=True) process_instance_event_id: int = db.Column(ForeignKey("process_instance_event.id"), nullable=False, index=True) - process_instance_event = relationship('ProcessInstanceEventModel') + process_instance_event = relationship('ProcessInstanceEventModel') # type: ignore message: str = db.Column(db.String(1024), nullable=False) # this should be 65k in mysql stacktrace: str = db.Column(db.Text(), nullable=False) + + task_line_number: Optional[int] = db.Column(db.Integer) + task_offset: Optional[int] = db.Column(db.Integer) + task_line_contents: Optional[str] = db.Column(db.String(255)) + task_trace: Optional[list] = db.Column(db.JSON) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py index 1e6833c6..58acee74 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -131,6 +131,7 @@ def process_instance_run( ProcessInstanceIsNotEnqueuedError, ProcessInstanceIsAlreadyLockedError, ) as e: + # import pdb; pdb.set_trace() ErrorHandlingService.handle_error(process_instance, e) raise e except Exception as e: @@ -138,6 +139,7 @@ def process_instance_run( # FIXME: this is going to point someone to the wrong task - it's misinformation for errors in sub-processes. # we need to recurse through all last tasks if the last task is a call activity or subprocess. if processor is not None: + # import pdb; pdb.set_trace() task = processor.bpmn_process_instance.last_task raise ApiError.from_task( error_code="unknown_exception", diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index d8123fd8..e965bdac 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -1,5 +1,7 @@ import copy import json +from spiffworkflow_backend.exceptions.api_error import ApiError +from SpiffWorkflow.exceptions import WorkflowTaskException # type: ignore from flask import g from spiffworkflow_backend.models.process_instance_error_detail import ProcessInstanceErrorDetailModel import traceback @@ -158,7 +160,7 @@ class TaskService: if task_model.state == "COMPLETED" or task_failed: event_type = ProcessInstanceEventType.task_completed.value - if task_failed: + if task_failed or task_model.state == TaskState.ERROR: event_type = ProcessInstanceEventType.task_failed.value # FIXME: some failed tasks will currently not have either timestamp since we only hook into spiff when tasks complete @@ -620,9 +622,24 @@ class TaskService: stacktrace = traceback.format_exc()[0:63999] message = str(exception)[0:1023] + task_line_number = None + task_line_contents = None + task_trace = None + task_offset = None + # import pdb; pdb.set_trace() + if isinstance(exception, WorkflowTaskException) or (isinstance(exception, ApiError) and exception.error_code == 'task_error'): + task_line_number = exception.line_number + task_line_contents = exception.error_line + task_trace = exception.task_trace + task_offset = exception.offset + process_instance_error_detail = ProcessInstanceErrorDetailModel( process_instance_event=process_instance_event, message=message, stacktrace=stacktrace, + task_line_number=task_line_number, + task_line_contents=task_line_contents, + task_trace=task_trace, + task_offset=task_offset, ) db.session.add(process_instance_error_detail) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py index 5764cc89..253c402e 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py @@ -138,6 +138,8 @@ class TaskModelSavingDelegate(EngineStepDelegate): | TaskState.MAYBE | TaskState.LIKELY | TaskState.FUTURE + | TaskState.STARTED + | TaskState.ERROR ): # these will be removed from the parent and then ignored if waiting_spiff_task._has_state(TaskState.PREDICTED_MASK): diff --git a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx index 411a9a53..dc973727 100644 --- a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx +++ b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx @@ -1,5 +1,6 @@ import { Notification } from './Notification'; import useAPIError from '../hooks/UseApiError'; +import { ErrorForDisplay } from '../interfaces'; function errorDetailDisplay( errorObject: any, @@ -18,57 +19,65 @@ function errorDetailDisplay( return null; } +export const childrenForErrorObject = (errorObject: ErrorForDisplay) => { + let sentryLinkTag = null; + if (errorObject.sentry_link) { + sentryLinkTag = ( + + { + ': Find details about this error here (it may take a moment to become available): ' + } + + {errorObject.sentry_link} + + + ); + } + + const message =
{errorObject.message}
; + const taskName = errorDetailDisplay(errorObject, 'task_name', 'Task Name'); + const taskId = errorDetailDisplay(errorObject, 'task_id', 'Task ID'); + const fileName = errorDetailDisplay(errorObject, 'file_name', 'File Name'); + const lineNumber = errorDetailDisplay( + errorObject, + 'line_number', + 'Line Number' + ); + const errorLine = errorDetailDisplay(errorObject, 'error_line', 'Context'); + let taskTrace = null; + if (errorObject.task_trace && errorObject.task_trace.length > 1) { + taskTrace = ( +
+ Call Activity Trace: + {errorObject.task_trace.reverse().join(' -> ')} +
+ ); + } + + return [ + message, +
, + sentryLinkTag, + taskName, + taskId, + fileName, + lineNumber, + errorLine, + taskTrace, + ]; +}; + export default function ErrorDisplay() { const errorObject = useAPIError().error; const { removeError } = useAPIError(); let errorTag = null; - if (errorObject) { - let sentryLinkTag = null; - if (errorObject.sentry_link) { - sentryLinkTag = ( - - { - ': Find details about this error here (it may take a moment to become available): ' - } - - {errorObject.sentry_link} - - - ); - } - const message =
{errorObject.message}
; + if (errorObject) { const title = 'Error:'; - const taskName = errorDetailDisplay(errorObject, 'task_name', 'Task Name'); - const taskId = errorDetailDisplay(errorObject, 'task_id', 'Task ID'); - const fileName = errorDetailDisplay(errorObject, 'file_name', 'File Name'); - const lineNumber = errorDetailDisplay( - errorObject, - 'line_number', - 'Line Number' - ); - const errorLine = errorDetailDisplay(errorObject, 'error_line', 'Context'); - let taskTrace = null; - if (errorObject.task_trace && errorObject.task_trace.length > 1) { - taskTrace = ( -
- Call Activity Trace: - {errorObject.task_trace.reverse().join(' -> ')} -
- ); - } errorTag = ( removeError()} type="error"> - {message} -
- {sentryLinkTag} - {taskName} - {taskId} - {fileName} - {lineNumber} - {errorLine} - {taskTrace} + <>{childrenForErrorObject(errorObject)} ); } diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts index c26c1ce3..7cf097d9 100644 --- a/spiffworkflow-frontend/src/interfaces.ts +++ b/spiffworkflow-frontend/src/interfaces.ts @@ -238,8 +238,9 @@ export interface ErrorForDisplay { task_name?: string; task_id?: string; line_number?: number; + error_line?: string; file_name?: string; - task_trace?: [string]; + task_trace?: string[]; } export interface AuthenticationParam { @@ -301,6 +302,10 @@ export interface ProcessInstanceEventErrorDetail { id: number; message: string; stacktrace: string; + task_line_contents?: string; + task_line_number?: number; + task_offset?: number; + task_trace?: string[]; } export interface ProcessInstanceLogEntry { diff --git a/spiffworkflow-frontend/src/routes/ProcessInstanceLogList.tsx b/spiffworkflow-frontend/src/routes/ProcessInstanceLogList.tsx index 16bb3f68..7f54efd1 100644 --- a/spiffworkflow-frontend/src/routes/ProcessInstanceLogList.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessInstanceLogList.tsx @@ -32,12 +32,14 @@ import { import HttpService from '../services/HttpService'; import { useUriListForPermissions } from '../hooks/UriListForPermissions'; import { + ErrorForDisplay, PermissionsToCheck, ProcessInstanceEventErrorDetail, ProcessInstanceLogEntry, } from '../interfaces'; import Filters from '../components/Filters'; import { usePermissionFetcher } from '../hooks/PermissionService'; +import { childrenForErrorObject } from '../components/ErrorDisplay'; type OwnProps = { variant: string; @@ -54,6 +56,7 @@ export default function ProcessInstanceLogList({ variant }: OwnProps) { const [taskTypes, setTaskTypes] = useState([]); const [eventTypes, setEventTypes] = useState([]); + const [eventForModal, setEventForModal] = useState(null); const [eventErrorDetails, setEventErrorDetails] = @@ -150,15 +153,25 @@ export default function ProcessInstanceLogList({ variant }: OwnProps) { const errorEventModal = () => { if (eventForModal) { - const modalHeading = `Event Error Details for`; + const modalHeading = 'Event Error Details'; let errorMessageTag = ( ); if (eventErrorDetails) { + const errorForDisplay: ErrorForDisplay = { + message: eventErrorDetails.message, + task_name: eventForModal.task_definition_name, + task_id: eventForModal.task_definition_identifier, + line_number: eventErrorDetails.task_line_number, + error_line: eventErrorDetails.task_line_contents, + task_trace: eventErrorDetails.task_trace, + }; + const errorChildren = childrenForErrorObject(errorForDisplay); errorMessageTag = ( <> -

{eventErrorDetails.message} NOOO

+

{eventErrorDetails.message}


+ {errorChildren} 
{eventErrorDetails.stacktrace}
);