Skip to content

Commit 480ac32

Browse files
authored
fix(flux-lsp/516): isolate the source of the runtime bugs, with the new schema injection (#4980)
We have many bugs (>3k in 2 days) being reported from the connector library used to connect our UI with our LSP worker thread. The connector library is swallowing the stacktrace, so we don't know the reason. Nor do we know yet if it's impacting users. Therefore the purpose of this PR is to increase the honeybadger error reporting. Add error reporting on the global scope of the worker thread itself, and add reporting on one possible upstream culprit.
1 parent 81f3f67 commit 480ac32

File tree

4 files changed

+51
-5
lines changed

4 files changed

+51
-5
lines changed

src/languageSupport/languages/flux/lsp/connection.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import {
1515
ExecuteCommandT,
1616
} from 'src/languageSupport/languages/flux/lsp/utils'
1717

18+
// error reporting
19+
import {reportErrorThroughHoneyBadger} from 'src/shared/utils/errors'
20+
1821
class LspConnectionManager {
1922
private _worker: Worker
2023
private _model: MonacoTypes.editor.IModel
@@ -69,15 +72,23 @@ class LspConnectionManager {
6972
command: ExecuteCommand,
7073
data: Omit<ExecuteCommandArgument, 'textDocument'>
7174
) {
72-
this._worker.postMessage(
73-
executeCommand([
75+
let msg
76+
try {
77+
msg = executeCommand([
7478
command,
7579
{
7680
...data,
7781
textDocument: {uri: this._model.uri.toString()},
7882
},
7983
] as ExecuteCommandT)
80-
)
84+
} catch (err) {
85+
console.error(`Error ExecuteCommand:`, err)
86+
reportErrorThroughHoneyBadger(err, {
87+
name: 'LSP injection payload',
88+
})
89+
return
90+
}
91+
this._worker.postMessage(msg)
8192
}
8293

8394
dispose() {

src/languageSupport/languages/flux/lsp/monaco.flux.lsp.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ export function initLspWorker() {
7575
} else {
7676
worker = new Fallback()
7777
}
78+
worker.onerror = (err: ErrorEvent) => {
79+
const error: Error = {...err, name: 'worker.onerror'}
80+
reportErrorThroughHoneyBadger(error, {name: 'LSP worker'})
81+
}
7882
manager = new ConnectionManager(worker)
7983

8084
messageReader = new BrowserMessageReader(worker)

src/languageSupport/languages/flux/lsp/utils.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,38 @@ export type ExecuteCommandT =
8383
| [ExecuteCommand.InjectTagValue, ExecuteCommandInjectTagValue]
8484
| [ExecuteCommand.InjectField, ExecuteCommandInjectField]
8585

86-
export const executeCommand = ([command, arg]: ExecuteCommandT) => {
86+
function validateExecuteCommandPayload([command, arg]: ExecuteCommandT):
87+
| boolean
88+
| never {
89+
const checkIsString = (arg, prop) => {
90+
if (!(arg[prop] && typeof arg[prop] === 'string')) {
91+
throw new Error(
92+
`${prop} should be type string, instead found: ${typeof arg[prop]}`
93+
)
94+
}
95+
return true
96+
}
97+
98+
switch (command) {
99+
case ExecuteCommand.InjectionMeasurement:
100+
return checkIsString(arg, 'bucket')
101+
case ExecuteCommand.InjectTag:
102+
case ExecuteCommand.InjectField:
103+
return checkIsString(arg, 'bucket') && checkIsString(arg, 'name')
104+
case ExecuteCommand.InjectTagValue:
105+
return (
106+
checkIsString(arg, 'bucket') &&
107+
checkIsString(arg, 'name') &&
108+
checkIsString(arg, 'value')
109+
)
110+
default:
111+
throw new Error(`unrecognized ExecuteCommand`)
112+
}
113+
}
114+
115+
export const executeCommand = (payload: ExecuteCommandT): object | never => {
116+
validateExecuteCommandPayload(payload)
117+
const [command, arg] = payload
87118
return createRequest(Methods.ExecuteCommand, {
88119
command,
89120
arguments: [arg],

src/languageSupport/languages/flux/lsp/worker/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ export const respond = (msg, cb) => {
33
const d = JSON.parse(msg)
44
cb(d)
55
} catch (e) {
6-
console.warn(e)
6+
console.error(e)
77
}
88
}

0 commit comments

Comments
 (0)