Skip to content

Commit edc4e8d

Browse files
bjones1bjones1
committed
Refactor: produce more coherent errors.
1 parent 033f31f commit edc4e8d

File tree

9 files changed

+205
-141
lines changed

9 files changed

+205
-141
lines changed

‎client/src/CodeChatEditorFramework.mts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
on_error,
4646
on_dom_content_loaded,
4747
} from "./CodeChatEditor.mjs";
48+
import { ResultErrTypes } from "./rust-types/ResultErrTypes.js";
4849

4950
// Websocket
5051
// -----------------------------------------------------------------------------
@@ -148,7 +149,12 @@ class WebSocketComm {
148149
) {
149150
const msg = `Ignoring update for ${current_update.file_path} because it's not the current file ${this.current_filename}.`;
150151
report_error(msg);
151-
this.send_result(id, msg);
152+
this.send_result(id, {
153+
IgnoredUpdate: [
154+
current_update.file_path,
155+
this.current_filename,
156+
],
157+
});
152158
return;
153159
}
154160
const contents = current_update.contents;
@@ -199,7 +205,7 @@ class WebSocketComm {
199205
);
200206
}
201207

202-
this.send_result(id, null);
208+
this.send_result(id);
203209
});
204210
break;
205211

@@ -234,7 +240,7 @@ class WebSocketComm {
234240
// `current_filename` should be set on the next `Update`
235241
// message.
236242
this.current_filename = undefined;
237-
this.send_result(id, null);
243+
this.send_result(id);
238244
});
239245
break;
240246

@@ -264,7 +270,11 @@ class WebSocketComm {
264270
value,
265271
)})`;
266272
report_error(msg);
267-
this.send_result(id, msg);
273+
this.send_result(id, {
274+
ClientIllegalMessageReceived: `${key}(${format_struct(
275+
value,
276+
)})`,
277+
});
268278
break;
269279
}
270280
};
@@ -348,9 +358,9 @@ class WebSocketComm {
348358

349359
// Send a result (a response to a message from the server) back to the
350360
// server.
351-
send_result = (id: number, result: string | null = null) => {
361+
send_result = (id: number, result?: ResultErrTypes) => {
352362
const message: EditorMessageContents = {
353-
Result: result === null ? { Ok: "Void" } : { Err: result },
363+
Result: result === undefined ? { Ok: "Void" } : { Err: result },
354364
};
355365
console_log(
356366
`CodeChat Client: sending result id = ${id}, message = ${format_struct(message)}`,

‎client/src/shared_types.mts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ import { StringDiff } from "./rust-types/StringDiff.js";
3131
import { CodeMirrorDocBlockTuple } from "./rust-types/CodeMirrorDocBlockTuple.js";
3232
import { UpdateMessageContents } from "./rust-types/UpdateMessageContents.js";
3333
import { ResultOkTypes } from "./rust-types/ResultOkTypes.js";
34+
import { ResultErrTypes } from "./rust-types/ResultErrTypes.js";
3435

3536
// Manually define this, since `ts-rs` can't export `webserver.MessageResult`.
36-
type MessageResult = { Ok: ResultOkTypes } | { Err: string };
37+
type MessageResult = { Ok: ResultOkTypes } | { Err: ResultErrTypes };
3738

3839
export type {
3940
EditorMessageContents,

‎server/src/ide.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ use crate::{
5151
translation::{CreatedTranslationQueues, create_translation_queues},
5252
webserver::{
5353
self, EditorMessage, EditorMessageContents, INITIAL_IDE_MESSAGE_ID, MESSAGE_ID_INCREMENT,
54-
REPLY_TIMEOUT_MS, ResultOkTypes, UpdateMessageContents, WebAppState, setup_server,
54+
REPLY_TIMEOUT_MS, ResultErrTypes, ResultOkTypes, UpdateMessageContents, WebAppState,
55+
setup_server,
5556
},
5657
};
5758

@@ -160,7 +161,7 @@ impl CodeChatEditorServer {
160161
Some(
161162
EditorMessage {
162163
id,
163-
message: EditorMessageContents::Result(Err(format!("Timeout: message id {id} unacknowledged.")))
164+
message: EditorMessageContents::Result(Err(webserver::ResultErrTypes::MessageTimeout(id)))
164165
}
165166
),
166167
else => None,
@@ -271,7 +272,7 @@ impl CodeChatEditorServer {
271272
.await
272273
}
273274

274-
// Send either an Ok(Void) or an Error result to the Client.
275+
/// Send either an Ok(Void) or an Error result to the Client.
275276
pub async fn send_result(
276277
&self,
277278
id: f64,
@@ -281,7 +282,7 @@ impl CodeChatEditorServer {
281282
id,
282283
message: webserver::EditorMessageContents::Result(
283284
if let Some(message_result) = message_result {
284-
Err(message_result)
285+
Err(ResultErrTypes::ExtensionError(message_result))
285286
} else {
286287
Ok(ResultOkTypes::Void)
287288
},

‎server/src/ide/filewatcher.rs‎

Lines changed: 22 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use crate::{
5757
processing::CodeMirrorDiffable,
5858
queue_send,
5959
webserver::{
60-
INITIAL_IDE_MESSAGE_ID, MESSAGE_ID_INCREMENT, ResultOkTypes, WebAppState,
60+
INITIAL_IDE_MESSAGE_ID, MESSAGE_ID_INCREMENT, ResultErrTypes, ResultOkTypes, WebAppState,
6161
filesystem_endpoint, get_test_mode,
6262
},
6363
};
@@ -463,12 +463,12 @@ async fn processing_task(
463463
for err in err_vec {
464464
// Report errors locally and to the CodeChat
465465
// Editor.
466-
let msg = format!("Watcher error: {err}");
467-
error!("{msg}");
466+
let err = ResultErrTypes::FileWatchingError(err.to_string());
467+
error!("{err:?}");
468468
// Send using an ID which indicates this isn't a
469469
// response to a message received from the
470470
// client.
471-
send_response(&from_ide_tx, RESERVED_MESSAGE_ID, Err(msg)).await;
471+
send_response(&from_ide_tx, RESERVED_MESSAGE_ID, Err(err)).await;
472472
}
473473
}
474474

@@ -556,10 +556,7 @@ async fn processing_task(
556556
// file. If `canonicalize` fails, then the files
557557
// don't match.
558558
if Some(Path::new(&update_message_contents.file_path).to_path_buf()) != current_filepath {
559-
break 'process Err(format!(
560-
"Update for file '{}' doesn't match current file '{current_filepath:?}'.",
561-
update_message_contents.file_path
562-
));
559+
break 'process Err(ResultErrTypes::WrongFileUpdate(update_message_contents.file_path, current_filepath.clone()));
563560
}
564561
// With code or a path, there's nothing to save.
565562
let codechat_for_web = match update_message_contents.contents {
@@ -578,26 +575,14 @@ async fn processing_task(
578575
// it, in order to avoid a watch notification
579576
// from this write.
580577
if let Err(err) = debounced_watcher.unwatch(cfp) {
581-
let msg = format!(
582-
"Unable to unwatch file '{}': {err}.",
583-
cfp.to_string_lossy()
584-
);
585-
break 'process Err(msg);
578+
break 'process Err(ResultErrTypes::FileUnwatchError(cfp.to_path_buf(), err.to_string()));
586579
}
587580
// Save this string to a file.
588581
if let Err(err) = fs::write(cfp.as_path(), plain.doc).await {
589-
let msg = format!(
590-
"Unable to save file '{}': {err}.",
591-
cfp.to_string_lossy()
592-
);
593-
break 'process Err(msg);
582+
break 'process Err(ResultErrTypes::SaveFileError(cfp.to_path_buf(), err.to_string()));
594583
}
595584
if let Err(err) = debounced_watcher.watch(cfp, RecursiveMode::NonRecursive) {
596-
let msg = format!(
597-
"Unable to watch file '{}': {err}.",
598-
cfp.to_string_lossy()
599-
);
600-
break 'process Err(msg);
585+
break 'process Err(ResultErrTypes::FileWatchError(cfp.to_path_buf(), err.to_string()));
601586
}
602587
Ok(ResultOkTypes::Void)
603588
};
@@ -611,19 +596,14 @@ async fn processing_task(
611596
if let Some(cfp) = &current_filepath
612597
&& let Err(err) = debounced_watcher.unwatch(cfp)
613598
{
614-
break 'err_exit Err(format!(
615-
"Unable to unwatch file '{}': {err}.",
616-
cfp.to_string_lossy()
617-
));
599+
break 'err_exit Err(ResultErrTypes::FileUnwatchError(cfp.to_path_buf(), err.to_string()));
618600
}
619601
// Update to the new path.
620602
current_filepath = Some(file_path.to_path_buf());
621603

622604
// Watch the new file.
623-
if let Err(err) = debounced_watcher.watch(file_path, RecursiveMode::NonRecursive) {
624-
break 'err_exit Err(format!(
625-
"Unable to watch file '{file_path_str}': {err}.",
626-
));
605+
if let Err(err) = debounced_watcher.watch(&file_path, RecursiveMode::NonRecursive) {
606+
break 'err_exit Err(ResultErrTypes::FileWatchError(file_path.to_path_buf(), err.to_string()));
627607
}
628608
// Indicate there was no error in the `Result`
629609
// message.
@@ -656,9 +636,9 @@ async fn processing_task(
656636
EditorMessageContents::OpenUrl(_) |
657637
EditorMessageContents::ClientHtml(_) |
658638
EditorMessageContents::RequestClose => {
659-
let msg = format!("Client sent unsupported message type {m:?}");
660-
error!("{msg}");
661-
send_response(&from_ide_tx, m.id, Err(msg)).await;
639+
let err = ResultErrTypes::ClientIllegalMessage;
640+
error!("{err:?}");
641+
send_response(&from_ide_tx, m.id, Err(err)).await;
662642
}
663643
}
664644
}
@@ -727,7 +707,6 @@ mod tests {
727707
dev::{Service, ServiceResponse},
728708
test,
729709
};
730-
use assertables::assert_starts_with;
731710
use dunce::simplified;
732711
use path_slash::PathExt;
733712
use pretty_assertions::assert_eq;
@@ -745,8 +724,8 @@ mod tests {
745724
webserver::{
746725
EditorMessage, EditorMessageContents, INITIAL_CLIENT_MESSAGE_ID,
747726
INITIAL_IDE_MESSAGE_ID, INITIAL_MESSAGE_ID, IdeType, MESSAGE_ID_INCREMENT,
748-
ResultOkTypes, UpdateMessageContents, WebAppState, WebsocketQueues, configure_app,
749-
drop_leading_slash, make_app_data, send_response, set_root_path,
727+
ResultErrTypes, ResultOkTypes, UpdateMessageContents, WebAppState, WebsocketQueues,
728+
configure_app, drop_leading_slash, make_app_data, send_response, set_root_path,
750729
},
751730
};
752731

@@ -963,7 +942,7 @@ mod tests {
963942
.unwrap();
964943
let (id_rx, msg_rx) = get_message_as!(to_client_rx, EditorMessageContents::Result);
965944
assert_eq!(id, id_rx);
966-
assert_starts_with!(cast!(&msg_rx, Err), "Client must not send this message.");
945+
matches!(cast!(&msg_rx, Err), ResultErrTypes::ClientIllegalMessage);
967946
}
968947

969948
// 5. Send an update message with no path.
@@ -993,10 +972,7 @@ mod tests {
993972
// Check that it produces an error.
994973
let (id, err_msg) = get_message_as!(to_client_rx, EditorMessageContents::Result);
995974
assert_eq!(id, INITIAL_CLIENT_MESSAGE_ID + 4.0 * MESSAGE_ID_INCREMENT);
996-
assert_starts_with!(
997-
cast!(err_msg, Err),
998-
"Update for file '' doesn't match current file"
999-
);
975+
cast!(cast!(err_msg, Err), ResultErrTypes::WrongFileUpdate, _a, _b);
1000976

1001977
// 6. Send an update message with unknown source language.
1002978
//
@@ -1023,13 +999,12 @@ mod tests {
1023999
.unwrap();
10241000

10251001
// Check that it produces an error.
1002+
let (msg_id, msg) = get_message_as!(to_client_rx, EditorMessageContents::Result);
10261003
assert_eq!(
1027-
get_message_as!(to_client_rx, EditorMessageContents::Result),
1028-
(
1029-
INITIAL_CLIENT_MESSAGE_ID + 5.0 * MESSAGE_ID_INCREMENT,
1030-
Err("Unable to translate to source: Invalid mode".to_string())
1031-
)
1004+
msg_id,
1005+
INITIAL_CLIENT_MESSAGE_ID + 5.0 * MESSAGE_ID_INCREMENT
10321006
);
1007+
cast!(cast!(msg, Err), ResultErrTypes::CannotTranslateCodeChat);
10331008

10341009
// 7. Send a valid message.
10351010
//

‎server/src/ide/vscode.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ use crate::{
4545
translation_task,
4646
},
4747
webserver::{
48-
EditorMessage, EditorMessageContents, IdeType, RESERVED_MESSAGE_ID, ResultOkTypes,
49-
WebAppState, client_websocket, escape_html, filesystem_endpoint, get_client_framework,
50-
get_server_url, html_wrapper, send_response,
48+
EditorMessage, EditorMessageContents, IdeType, RESERVED_MESSAGE_ID, ResultErrTypes,
49+
ResultOkTypes, WebAppState, client_websocket, escape_html, filesystem_endpoint,
50+
get_client_framework, get_server_url, html_wrapper, send_response,
5151
},
5252
};
5353

@@ -125,9 +125,10 @@ pub fn vscode_ide_core(
125125

126126
// Make sure it's the `Opened` message.
127127
let EditorMessageContents::Opened(ide_type) = first_message.message else {
128-
let msg = format!("Unexpected message {first_message:?}");
129-
error!("{msg}");
130-
send_response(&to_ide_tx, first_message.id, Err(msg)).await;
128+
let id = first_message.id;
129+
let err = ResultErrTypes::UnexpectedMessage(format!("{:#?}", first_message));
130+
error!("{err}");
131+
send_response(&to_ide_tx, id, Err(err)).await;
131132

132133
// Send a `Closed` message to shut down the websocket.
133134
queue_send!(to_ide_tx.send(EditorMessage { id: RESERVED_MESSAGE_ID, message: EditorMessageContents::Closed}), 'task);
@@ -214,9 +215,9 @@ pub fn vscode_ide_core(
214215
if let Err(err) =
215216
webbrowser::open(&format!("{address}/vsc/cf/{connection_id_raw}"))
216217
{
217-
let msg = format!("Unable to open web browser: {err}");
218-
error!("{msg}");
219-
send_response(&to_ide_tx, first_message.id, Err(msg)).await;
218+
let err = ResultErrTypes::WebBrowserOpenFailed(err.to_string());
219+
error!("{err:?}");
220+
send_response(&to_ide_tx, first_message.id, Err(err)).await;
220221

221222
// Send a `Closed` message.
222223
queue_send!(to_ide_tx.send(EditorMessage{
@@ -231,9 +232,9 @@ pub fn vscode_ide_core(
231232
}
232233
_ => {
233234
// This is the wrong IDE type. Report the error.
234-
let msg = format!("Invalid IDE type: {ide_type:?}");
235-
error!("{msg}");
236-
send_response(&to_ide_tx, first_message.id, Err(msg)).await;
235+
let err = ResultErrTypes::InvalidIdeType(ide_type);
236+
error!("{err:?}");
237+
send_response(&to_ide_tx, first_message.id, Err(err)).await;
237238

238239
// Close the connection.
239240
queue_send!(to_ide_tx.send(EditorMessage { id: RESERVED_MESSAGE_ID, message: EditorMessageContents::Closed}), 'task);

‎server/src/ide/vscode/tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use tokio_tungstenite::{
4949

5050
use crate::webserver::{
5151
EditorMessage, EditorMessageContents, INITIAL_CLIENT_MESSAGE_ID, INITIAL_IDE_MESSAGE_ID,
52-
INITIAL_MESSAGE_ID, IdeType, MESSAGE_ID_INCREMENT,
52+
INITIAL_MESSAGE_ID, IdeType, MESSAGE_ID_INCREMENT, ResultErrTypes,
5353
};
5454
use crate::{
5555
cast,
@@ -268,7 +268,7 @@ async fn test_vscode_ide_websocket1() {
268268
let em = read_message(&mut ws_ide).await;
269269
let result = cast!(em.message, EditorMessageContents::Result);
270270

271-
assert_starts_with!(cast!(&result, Err), "Unexpected message");
271+
matches!(cast!(&result, Err), ResultErrTypes::ClientIllegalMessage);
272272

273273
// Next, expect the websocket to be closed.
274274
let err = &ws_ide.next().await.unwrap().unwrap();

0 commit comments

Comments
 (0)