diff --git a/.gitattributes b/.gitattributes index ba715681..dd2c4ca0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ -*.lua linguist-language=Luau \ No newline at end of file +*.lua linguist-language=Luau +rojo-test/**/*.txt text eol=lf \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c090788..9d73b5e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) +* Fixed server crash when deleting folders ([#1220]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -48,6 +49,7 @@ Making a new release? Simply add the new header with the version and date undern [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 +[#1220]: https://github.com/rojo-rbx/rojo/pull/1220 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap new file mode 100644 index 00000000..f5203d43 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap @@ -0,0 +1,16 @@ +--- +source: tests/tests/serve.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: [] + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: remove_folder + Parent: "00000000000000000000000000000000" + Properties: {} +messageCursor: 1 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap new file mode 100644 index 00000000..d861c560 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap @@ -0,0 +1,38 @@ +--- +source: tests/tests/serve.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: remove_folder + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: + - id-4 + ClassName: Folder + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: my-folder + Parent: id-2 + Properties: {} + id-4: + Children: [] + ClassName: StringValue + Id: id-4 + Metadata: + ignoreUnknownInstances: false + Name: test + Parent: id-3 + Properties: + Value: + String: "This file is inside my-folder\n" +messageCursor: 0 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap new file mode 100644 index 00000000..6796e4ec --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap @@ -0,0 +1,13 @@ +--- +source: tests/tests/serve.rs +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: remove_folder +protocolVersion: 5 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 +unexpectedPlaceIds: ~ diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap new file mode 100644 index 00000000..e9e39e04 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap @@ -0,0 +1,13 @@ +--- +source: tests/tests/serve.rs +expression: "socket_packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 1 + messages: + - added: {} + removed: + - id-3 + updated: [] +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-tests/remove_folder/default.project.json b/rojo-test/serve-tests/remove_folder/default.project.json new file mode 100644 index 00000000..28d0bc72 --- /dev/null +++ b/rojo-test/serve-tests/remove_folder/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "remove_folder", + "tree": { + "$path": "src" + } +} diff --git a/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt b/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt new file mode 100644 index 00000000..44c07d0f --- /dev/null +++ b/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt @@ -0,0 +1 @@ +This file is inside my-folder diff --git a/src/change_processor.rs b/src/change_processor.rs index 6c5c8224..78ca2858 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -172,12 +172,23 @@ impl JobThreadContext { self.apply_patches(self.vfs.canonicalize(&path).unwrap()) } VfsEvent::Remove(path) => { - // MemoFS does not track parent removals yet, so we can canonicalize - // the parent path safely and then append the removed path's file name. + // Try to canonicalize the parent path and append the removed path's + // file name. If the parent no longer exists (e.g., a folder and its + // contents were deleted together), we skip processing since the + // parent's removal event will handle cleaning up the instances. let parent = path.parent().unwrap(); let file_name = path.file_name().unwrap(); - let parent_normalized = self.vfs.canonicalize(parent).unwrap(); - self.apply_patches(parent_normalized.join(file_name)) + match self.vfs.canonicalize(parent) { + Ok(parent_normalized) => self.apply_patches(parent_normalized.join(file_name)), + Err(err) => { + log::debug!( + "Parent path no longer exists for removed path {}: {}", + path.display(), + err + ); + Vec::new() + } + } } _ => { log::warn!("Unhandled VFS event: {:?}", event); diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 748f1687..d41ee72e 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -126,6 +126,41 @@ fn remove_file() { }); } +/// Test that removing a folder (directory) with contents doesn't crash the server. +/// This is a regression test for a bug where deleting a folder would cause a panic +/// due to the parent path no longer existing when processing child removal events. +#[test] +fn remove_folder() { + run_serve_test("remove_folder", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!("remove_folder_info", redactions.redacted_yaml(info)); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "remove_folder_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + + fs::remove_dir_all(session.path().join("src/my-folder")).unwrap(); + + let socket_packet = session + .get_api_socket_packet(SocketPacketType::Messages, 0) + .unwrap(); + assert_yaml_snapshot!( + "remove_folder_subscribe", + socket_packet.intern_and_redact(&mut redactions, ()) + ); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "remove_folder_all-2", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} + #[test] fn edit_init() { run_serve_test("edit_init", |session, mut redactions| {