fix: run error handling, rollback visibility, stale status indicator, and spurious notifications

[abhizipstack]

What

Fixes 4 bugs reported from QA staging testing:

• Generic "Check server logs" error message on model run failure instead of actual error details
• Rollback button never appearing in the failure notification toast
• Stale failure indicator (red dot) in explorer persisting after user fixes the model
• Spurious "Something went wrong" popups triggered by JS runtime errors (not API failures)

Why

• Users had no way to know what went wrong when a model run failed — they were told to "check server logs" which they don't have access to
• The Rollback button was implemented but never visible due to is_rollback being nested inside message_args instead of at the top level of the error response
• After deleting a bad transformation, users were confused by the stale red dot still showing the old error
• Random "Something went wrong" popups with no useful info were eroding user trust

How

Bug 1 — Generic error message (backend/backend/core/routers/execute/views.py):

• Split the blanket except Exception into two: VisitranBaseExceptions/VisitranBackendBaseException (returns err.error_response() with detailed markdown error) and generic Exception (returns 500 with clearer fallback message)
• Uses err.to_response() when available to preserve the exception's own HTTP status code

Bug 2 — Rollback button (backend/visitran/errors/base_exceptions.py, backend/backend/errors/visitran_backend_base_exceptions.py):

• Both base exception classes now surface is_rollback at the top level of error_response() when present in _msg_args, matching what the frontend expects at error.response.data.is_rollback

Bug 3 — Stale failure indicator (backend/backend/application/context/no_code_model.py):

• Added _reset_model_run_status() called from _validate_and_update_model() to clear FAILED status back to NOT_STARTED when the model spec changes
• Guarded by config_type so presentation-only changes (sort/hide/order) do NOT reset the status

Bug 4 — Spurious popups (frontend/src/service/notification-service.js):

• Added guard to skip notification when error is a JS runtime error (no .response and no .request) with no explicit message — logs to console instead
• Preserves notifications for Axios network errors (which set .request)

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

No. All changes are backward-compatible:

• The error response structure is a superset of the previous one (adds is_rollback at top level, keeps message_args intact)
• The handle_http_request decorator already sets is_rollback at the top level for other endpoints — this aligns the run endpoint behavior
• Status reset only triggers on FAILED state, does not affect SUCCESS or RUNNING
• The notification guard only suppresses errors that previously showed a useless "Something went wrong" popup

Database Migrations

None

Env Config

None

Relevant Docs

Related Issues or PRs

Dependencies Versions

No changes

Notes on Testing

• Trigger a model run with a bad transformation (e.g., LN(0)) — verify error toast shows specific error with model name, not "Check server logs"
• On run failure for a model that had a previous successful run — verify Rollback button appears in the error notification
• Click Rollback — verify model reverts to previous spec and re-runs successfully
• After a failed run, delete the bad transformation — verify the red dot disappears from the explorer
• Change presentation (sort/hide column) on a failed model — verify red dot stays
• Add a new transformation and run again — verify green dot shows on success
• Check browser console for JS errors that previously triggered "Something went wrong" — verify no popup, only console log
• Disconnect network briefly — verify "Failed" notification still appears (not suppressed)

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

[greptile-apps]

Greptile Summary

This PR fixes four QA-reported bugs: surfacing real error details on model run failure, making the Rollback button visible in the error toast, clearing stale failure indicators after execution-affecting spec changes, and suppressing spurious JS runtime error popups. All four previous reviewer concerns (network-error suppression, status-code preservation, presentation-only reset guard, DB-save exception isolation) have been resolved in follow-up commits.

Confidence Score: 5/5

Safe to merge — all prior P0/P1 findings are addressed and no new issues were found.

All four bugs are fixed with targeted, backward-compatible changes. Each previous reviewer concern (network-error guard, HTTP status preservation, presentation-only guard, DB exception isolation) has been addressed in follow-up commits. No new logic or data-integrity issues introduced.

No files require special attention.

Important Files Changed

Filename	Overview
backend/backend/application/context/no_code_model.py	Adds _reset_model_run_status guarded by NON_EXECUTION_CONFIG_TYPES and broad except Exception so status reset never poisons the outer update result. Previous thread concerns fully addressed.
backend/backend/core/routers/execute/views.py	Exception handling split into domain exceptions (preserving HTTP status via to_response()) and fallback except Exception returning 500. Previous status-code concern addressed.
backend/backend/errors/visitran_backend_base_exceptions.py	Surfaces is_rollback at top level of error_response() when present in _msg_args; backward-compatible since message_args is still included.
backend/visitran/errors/base_exceptions.py	Same is_rollback promotion as the backend base exception; mirrors the sibling class change correctly.

Sequence Diagram

sequenceDiagram
    participant FE as Frontend
    participant View as execute_run_command
    participant App as ApplicationContext
    participant NC as NoCodeModel
    participant DB as ConfigModels (DB)

    FE->>View: POST /execute/run
    View->>App: execute_visitran_run_command()
    alt Success
        App-->>View: OK
        View->>App: backup_current_no_code_model()
        View-->>FE: 200 {status: success}
    else VisitranBaseExceptions / VisitranBackendBaseException
        App-->>View: raises domain exception
        View->>View: err.to_response() or err.error_response()
        View-->>FE: 4xx/5xx with error_message + is_rollback (top-level)
    else Unexpected Exception
        App-->>View: raises generic Exception
        View-->>FE: 500 {status: failed, error_message: Unexpected error}
    end

    FE->>View: PUT /model (spec change)
    View->>NC: _validate_and_update_model(config_type)
    NC->>NC: update_model()
    alt config_type not in NON_EXECUTION_CONFIG_TYPES
        NC->>DB: get model instance
        DB-->>NC: model_instance
        alt run_status == FAILED
            NC->>DB: save(run_status=NOT_STARTED, failure_reason=None)
        end
    end
    NC-->>View: result
    View-->>FE: updated model

Loading

_{Reviews (5): Last reviewed commit: "fix: catch all exceptions in _reset_mode..." | Re-trigger Greptile}

[wicky-zipstack]

@abhizipstack 4 distinct bugs each with a clear root cause and minimal fix.

views.py — error handling split
Good. The existing exception classes already have formatted error messages — no reason to discard them with a generic catch-all. Minor: the hasattr(err, 'to_response') check on L70 — if no exception class implements to_response(), this is dead code. Consider removing it.

base_exceptions.py / visitran_backend_base_exceptions.py — rollback visibility
Clean fix. Frontend already expects is_rollback at top level — this surfaces it without changing the API contract.

no_code_model.py — stale failure reset
Logic is sound — only resets FAILED → NOT_STARTED, leaves SUCCESS/RUNNING alone. The config_type not in ("presentation",) guard is a nice touch. Minor: consider a named constant like NON_EXECUTION_CONFIG_TYPES = {"presentation"} so it's more discoverable if more types get added later.

notification-service.js — runtime error suppression
The !error.request check correctly distinguishes JS runtime errors from network errors (which have .request set by Axios). Network failures will still show a notification. Looks correct.

[abhizipstack]

@wicky-zipstack Thanks for the review!

hasattr(err, 'to_response') — Keeping it as-is since VisitranBaseExceptions doesn't have to_response(), only VisitranBackendBaseException does. The guard is needed to handle both exception types in the same except block.

NON_EXECUTION_CONFIG_TYPES constant — Applied in fe74614. Extracted as a module-level set for discoverability.

FE notification change — Reverted from this PR, will handle separately.

[tahierhussain]

LGTM