Parcourir la source

fix: a published run recorded no workflow id, and an editor could not recognise its own lock

Two faults, both of which made something look broken that had worked.

A run of a published workflow recorded an empty workflowId, so the
workflow's own execution list showed nothing since the day it was
published - it read as "it has not run for six hours" while it was in fact
running every few minutes. Every trigger run of a published workflow was
affected, not only webhooks, which is what the earlier note about webhook
executions was really seeing.

The cause is an asymmetry worth knowing: reading a document gives its
metadata - _id, _version, timestamps - and reading a stored version of the
same document gives none of them. A version cannot say which document it
belongs to or even which version it is, though listVersions knows both.
The runner now stamps the id it asked for, which is right whatever the
database decides to return.

Separately, double-clicking a node refused to open its configuration and
flashed the holder's name - your own. The editor learns its connection id
from the auth_success message, and that arrives once: opening the editor by
navigating from another page means the socket authenticated long before,
so the message had already been and gone and the id stayed null. The
editor could not tell its own lock from somebody else's and refused to
open a node it was holding itself. It only worked after a hard refresh,
where the socket connects after the page mounts.

The id is kept on the WebSocket client now rather than in whichever
component happened to be listening. The same fault also stopped an editor
recognising its own live edits, so every change it made was echoed back
and applied to its own canvas.

Verified: a published workflow triggered by webhook records its id and
appears when the list is filtered by it; and double-clicking a node opens
the configuration when the editor is reached by navigation, which is the
path that failed.
fszontagh il y a 1 mois
Parent
commit
86ed2cfc5b
3 fichiers modifiés avec 29 ajouts et 12 suppressions
  1. 7 0
      src/runner/runner_service.cpp
  2. 13 2
      webui/src/api/client.ts
  3. 9 10
      webui/src/hooks/useWorkflowLocks.ts

+ 7 - 0
src/runner/runner_service.cpp

@@ -99,6 +99,13 @@ grpc::Status RunnerServiceImpl::ExecuteWorkflow(grpc::ServerContext* context,
             if (pinned.ok()) {
                 LOG_INFO("Workflow {} running published version {} (draft is {})",
                          request->workflow_id(), published, current);
+                // A stored version is the document as it was, and the database
+                // keeps _id outside the document - so a version comes back
+                // without one. Everything downstream identifies the run by it,
+                // and without it the execution recorded an empty workflowId:
+                // the run happened, but the workflow's own execution list never
+                // showed it, which reads as "it did not run".
+                pinned.value()["_id"] = request->workflow_id();
                 workflow_result = pinned;
             } else {
                 // Refusing would stop a live workflow because its history has

+ 13 - 2
webui/src/api/client.ts

@@ -80,6 +80,14 @@ export class WebSocketClient {
   // open, so a page that subscribes while connecting - which is every page,
   // on a cold load - was silently never subscribed to anything.
   private desiredChannels = new Set<string>()
+
+  // This connection's own id, told to us when the server accepts the token.
+  //
+  // Kept here rather than in whichever component happened to be listening:
+  // auth_success arrives once, and a page navigated to later has missed it.
+  // Something that needs to recognise its own messages - a node lock, say -
+  // would then never know which ones were its own.
+  clientId: string | null = null
   private reconnectAttempts: number = 0
   private maxReconnectAttempts: number = 5
   private baseReconnectDelay: number = 1000
@@ -147,8 +155,11 @@ export class WebSocketClient {
 
         // Subscriptions are replayed here rather than in onopen because the
         // server refuses a subscribe until it has accepted the token.
-        if (message.type === 'auth_success' && this.desiredChannels.size > 0) {
-          this.send({ type: 'subscribe', channels: [...this.desiredChannels] })
+        if (message.type === 'auth_success') {
+          this.clientId = message.clientId || null
+          if (this.desiredChannels.size > 0) {
+            this.send({ type: 'subscribe', channels: [...this.desiredChannels] })
+          }
         }
 
         const channel = message.channel || message.type

+ 9 - 10
webui/src/hooks/useWorkflowLocks.ts

@@ -51,7 +51,12 @@ export function useWorkflowLocks(
 
   // This connection's own id, so a lock held by this editor is distinguishable
   // from one held by the same person in a different tab.
-  const clientIdRef = useRef<string | null>(null)
+  //
+  // Read from the client rather than caught here: auth_success arrives once,
+  // and an editor opened by navigation rather than a page load has missed it.
+  // Missing it meant this editor could not recognise its own lock and refused
+  // to open a node it was itself holding, flashing the holder's name - your own.
+  const myClientId = () => wsClient.clientId
   const lastMoveSentRef = useRef(0)
   const heldRef = useRef<Set<string>>(new Set())
 
@@ -71,7 +76,7 @@ export function useWorkflowLocks(
     const onMove = (data: any) => {
       // Your own movement is already on your own canvas; echoing it back would
       // fight the drag in progress.
-      if (data?.clientId && data.clientId === clientIdRef.current) return
+      if (data?.clientId && data.clientId === myClientId()) return
       const p = data?.position
       if (!data?.nodeId || typeof p?.x !== 'number' || typeof p?.y !== 'number') return
       setLivePositions((current) => ({ ...current, [data.nodeId]: { x: p.x, y: p.y } }))
@@ -79,7 +84,7 @@ export function useWorkflowLocks(
 
     const onEdit = (data: any) => {
       // Your own edit is already on your own canvas.
-      if (data?.clientId && data.clientId === clientIdRef.current) return
+      if (data?.clientId && data.clientId === myClientId()) return
       onRemoteEditRef.current?.(data)
     }
 
@@ -88,15 +93,10 @@ export function useWorkflowLocks(
       setDeniedNode({ nodeId: data.nodeId, heldBy: data.heldBy || 'somebody else' })
     }
 
-    const onIdentity = (data: any) => {
-      if (data?.clientId) clientIdRef.current = data.clientId
-    }
-
     wsClient.on(lockChannel, onLocks)
     wsClient.on(moveChannel, onMove)
     wsClient.on(editChannel, onEdit)
     wsClient.on('lock_denied', onDenied)
-    wsClient.on('auth_success', onIdentity)
     wsClient.subscribe([lockChannel, moveChannel, editChannel])
 
     return () => {
@@ -113,7 +113,6 @@ export function useWorkflowLocks(
       wsClient.off(moveChannel, onMove)
       wsClient.off(editChannel, onEdit)
       wsClient.off('lock_denied', onDenied)
-      wsClient.off('auth_success', onIdentity)
       setLocks({})
       setLivePositions({})
       setDeniedNode(null)
@@ -156,7 +155,7 @@ export function useWorkflowLocks(
     (nodeId: string): NodeLock | null => {
       const held = locks[nodeId]
       if (!held) return null
-      return held.clientId === clientIdRef.current ? null : held
+      return held.clientId === myClientId() ? null : held
     },
     [locks]
   )