Skip to content

Commit

Permalink
Y-docs: Fix write capability recovery (#10851)
Browse files Browse the repository at this point in the history
When receiving no write capability in `text/openFile` response, the method returned error without synchronizing, but session state stayed at 'Opening' - this resulted in automatic success on the next retry without actually trying.

Added additional state to handle recovery from missing write capability errors.

# Important Notes
tested by applying the patch below (which simulates problem reported by James):

```diff
diff --git a/app/ydoc-shared/src/languageServer.ts b/app/ydoc-shared/src/languageServer.ts
index e1403f50420247575b437df8c603e5f1701b5e1d..753bbf9f9f449f6130c79040e44607ce0c308f7f 100644
--- a/app/ydoc-shared/src/languageServer.ts
+++ b/app/ydoc-shared/src/languageServer.ts
@@ -3,6 +3,7 @@ import { bytesToHex } from '@noble/hashes/utils'
import { Client, RequestManager } from '@open-rpc/client-js'
import debug from 'debug'
import { ObservableV2 } from 'lib0/observable'
+import { wait } from 'lib0/promise.js'
import { uuidv4 } from 'lib0/random'
import { z } from 'zod'
import { walkFs } from './languageServer/files'
@@ -268,9 +269,25 @@ export class LanguageServer extends ObservableV2<Notifications & TransportEvents
return this.request('session/initProtocolConnection', { clientId }, false)
}

+  private openCalls = 0
/** [Documentation](https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#textopenfile) */
openTextFile(path: Path): Promise<LsRpcResult<response.OpenTextFile>> {
-    return this.request<response.OpenTextFile>('text/openFile', { path })
+    if (this.openCalls === 0) {
+      this.openCalls = 1
+      return wait(1000).then(() =>
+        Err(new LsRpcError('Simluated timeout', 'text/openFile', { path })),
+      )
+    } else if (this.openCalls === 1) {
+      this.openCalls = 2
+      return this.request<response.OpenTextFile>('text/openFile', { path }).then(value => {
+        if (value.ok) {
+          value.value.writeCapability = null
+        }
+        return value
+      })
+    } else {
+      return this.request<response.OpenTextFile>('text/openFile', { path })
+    }
}

/** [Documentation](https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#textclosefile) */
```
  • Loading branch information
farmaazon authored Aug 20, 2024
1 parent 95e50f4 commit 22263e8
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions app/ydoc-server/src/languageServerSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ enum LsSyncState {
Synchronized,
WritingFile,
WriteError,
CapabilityError,
Reloading,
Closing,
Disposed,
Expand Down Expand Up @@ -402,6 +403,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
const result = await promise
if (!result.ok) return result
if (!result.value.writeCapability) {
this.setState(LsSyncState.CapabilityError)
return Err(
`Could not acquire write capability for module '${this.path.segments.join('/')}'`,
)
Expand All @@ -410,6 +412,8 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
return Ok()
})
}
case LsSyncState.CapabilityError:
return this.reload()
default: {
assertNever(this.state)
}
Expand Down Expand Up @@ -655,6 +659,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
}
return
}
case LsSyncState.CapabilityError:
case LsSyncState.WriteError:
case LsSyncState.Synchronized: {
this.setState(LsSyncState.Closing)
Expand Down Expand Up @@ -703,6 +708,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
return Ok()
})
}
case LsSyncState.CapabilityError:
case LsSyncState.WriteError: {
return this.withState(LsSyncState.Reloading, async () => {
const path = this.path.segments.join('/')
Expand Down

0 comments on commit 22263e8

Please sign in to comment.