fix: Don't halt and catch fire on deserialization errors in MSC4133 migration

conduwuit-example.toml

@@ -586,13 +586,10 @@
 #allow_unstable_room_versions = true
 
 # Default room version continuwuity will create rooms with.
-# Note that this has to be a string since the room version is a string
-# rather than an integer. Forgetting the quotes will make the server fail
-# to start!
 #
-# Per spec, room version "11" is the default.
+# Per spec, room version 11 is the default.
 #
-#default_room_version = "11"
+#default_room_version = 11
 
 # Enable OpenTelemetry OTLP tracing export. This replaces the deprecated
 # Jaeger exporter. Traces will be sent via OTLP to a collector (such as

nix/tests/default.nix

@@ -6,69 +6,6 @@
       pkgs,
       ...
     }:
-    let
-      baseTestScript =
-        pkgs.writers.writePython3Bin "do_test" { libraries = [ pkgs.python3Packages.matrix-nio ]; }
-          ''
-            import asyncio
-            import nio
-
-
-            async def main() -> None:
-                # Connect to continuwuity
-                client = nio.AsyncClient("http://continuwuity:6167", "alice")
-
-                # Register as user alice
-                response = await client.register("alice", "my-secret-password")
-
-                # Log in as user alice
-                response = await client.login("my-secret-password")
-
-                # Create a new room
-                response = await client.room_create(federate=False)
-                print("Matrix room create response:", response)
-                assert isinstance(response, nio.RoomCreateResponse)
-                room_id = response.room_id
-
-                # Join the room
-                response = await client.join(room_id)
-                print("Matrix join response:", response)
-                assert isinstance(response, nio.JoinResponse)
-
-                # Send a message to the room
-                response = await client.room_send(
-                    room_id=room_id,
-                    message_type="m.room.message",
-                    content={
-                        "msgtype": "m.text",
-                        "body": "Hello continuwuity!"
-                    }
-                )
-                print("Matrix room send response:", response)
-                assert isinstance(response, nio.RoomSendResponse)
-
-                # Sync responses
-                response = await client.sync(timeout=30000)
-                print("Matrix sync response:", response)
-                assert isinstance(response, nio.SyncResponse)
-
-                # Check the message was received by continuwuity
-                last_message = response.rooms.join[room_id].timeline.events[-1].body
-                assert last_message == "Hello continuwuity!"
-
-                # Leave the room
-                response = await client.room_leave(room_id)
-                print("Matrix room leave response:", response)
-                assert isinstance(response, nio.RoomLeaveResponse)
-
-                # Close the client
-                await client.close()
-
-
-            if __name__ == "__main__":
-                asyncio.run(main())
-          '';
-    in
     {
       # run some nixos tests as checks
       checks = lib.pipe self'.packages [
@@ -81,69 +18,106 @@
         # this test was initially yoinked from
         #
         # https://github.com/NixOS/nixpkgs/blob/960ce26339661b1b69c6f12b9063ca51b688615f/nixos/tests/matrix/continuwuity.nix
-        (builtins.concatMap (
-          name:
-          builtins.map
-            (
-              { config, suffix }:
-              {
-                name = "test-${name}-${suffix}";
-                value = pkgs.testers.runNixOSTest {
-                  inherit name;
+        (builtins.map (name: {
+          name = "test-${name}";
+          value = pkgs.testers.runNixOSTest {
+            inherit name;
 
-                  nodes = {
-                    continuwuity = {
-                      services.matrix-continuwuity = {
-                        enable = true;
-                        package = self'.packages.${name};
-                        settings = config;
-                        extraEnvironment.RUST_BACKTRACE = "yes";
-                      };
-                      networking.firewall.allowedTCPPorts = [ 6167 ];
-                    };
-                    client.environment.systemPackages = [ baseTestScript ];
-                  };
-
-                  testScript = ''
-                    start_all()
-
-                    with subtest("start continuwuity"):
-                          continuwuity.wait_for_unit("continuwuity.service")
-                          continuwuity.wait_for_open_port(6167)
-
-                    with subtest("ensure messages can be exchanged"):
-                          client.succeed("${lib.getExe baseTestScript} >&2")
-                  '';
-
-                };
-              }
-            )
-            [
-              {
-                suffix = "base";
-                config = {
-                  global = {
+            nodes = {
+              continuwuity = {
+                services.matrix-continuwuity = {
+                  enable = true;
+                  package = self'.packages.${name};
+                  settings.global = {
                     server_name = name;
                     address = [ "0.0.0.0" ];
                     allow_registration = true;
                     yes_i_am_very_very_sure_i_want_an_open_registration_server_prone_to_abuse = true;
                   };
+                  extraEnvironment.RUST_BACKTRACE = "yes";
                 };
-              }
-              {
-                suffix = "with-room-version";
-                config = {
-                  global = {
-                    server_name = name;
-                    address = [ "0.0.0.0" ];
-                    allow_registration = true;
-                    yes_i_am_very_very_sure_i_want_an_open_registration_server_prone_to_abuse = true;
-                    default_room_version = "12";
-                  };
+                networking.firewall.allowedTCPPorts = [ 6167 ];
+              };
+              client =
+                { pkgs, ... }:
+                {
+                  environment.systemPackages = [
+                    (pkgs.writers.writePython3Bin "do_test" { libraries = [ pkgs.python3Packages.matrix-nio ]; } ''
+                      import asyncio
+                      import nio
+
+
+                      async def main() -> None:
+                          # Connect to continuwuity
+                          client = nio.AsyncClient("http://continuwuity:6167", "alice")
+
+                          # Register as user alice
+                          response = await client.register("alice", "my-secret-password")
+
+                          # Log in as user alice
+                          response = await client.login("my-secret-password")
+
+                          # Create a new room
+                          response = await client.room_create(federate=False)
+                          print("Matrix room create response:", response)
+                          assert isinstance(response, nio.RoomCreateResponse)
+                          room_id = response.room_id
+
+                          # Join the room
+                          response = await client.join(room_id)
+                          print("Matrix join response:", response)
+                          assert isinstance(response, nio.JoinResponse)
+
+                          # Send a message to the room
+                          response = await client.room_send(
+                              room_id=room_id,
+                              message_type="m.room.message",
+                              content={
+                                  "msgtype": "m.text",
+                                  "body": "Hello continuwuity!"
+                              }
+                          )
+                          print("Matrix room send response:", response)
+                          assert isinstance(response, nio.RoomSendResponse)
+
+                          # Sync responses
+                          response = await client.sync(timeout=30000)
+                          print("Matrix sync response:", response)
+                          assert isinstance(response, nio.SyncResponse)
+
+                          # Check the message was received by continuwuity
+                          last_message = response.rooms.join[room_id].timeline.events[-1].body
+                          assert last_message == "Hello continuwuity!"
+
+                          # Leave the room
+                          response = await client.room_leave(room_id)
+                          print("Matrix room leave response:", response)
+                          assert isinstance(response, nio.RoomLeaveResponse)
+
+                          # Close the client
+                          await client.close()
+
+
+                      if __name__ == "__main__":
+                          asyncio.run(main())
+                    '')
+                  ];
                 };
-              }
-            ]
-        ))
+            };
+
+            testScript = ''
+              start_all()
+
+              with subtest("start continuwuity"):
+                    continuwuity.wait_for_unit("continuwuity.service")
+                    continuwuity.wait_for_open_port(6167)
+
+              with subtest("ensure messages can be exchanged"):
+                    client.succeed("do_test >&2")
+            '';
+
+          };
+        }))
         builtins.listToAttrs
       ];
     };

src/core/config/mod.rs

@@ -703,13 +703,10 @@ pub struct Config {
 	pub allow_unstable_room_versions: bool,
 
 	/// Default room version continuwuity will create rooms with.
-	/// Note that this has to be a string since the room version is a string
-	/// rather than an integer. Forgetting the quotes will make the server fail
-	/// to start!
 	///
-	/// Per spec, room version "11" is the default.
+	/// Per spec, room version 11 is the default.
 	///
-	/// default: "11"
+	/// default: 11
 	#[serde(default = "default_default_room_version")]
 	pub default_room_version: RoomVersionId,
 

src/service/migrations.rs

@@ -590,6 +590,10 @@ async fn fix_readreceiptid_readreceipt_duplicates(services: &Services) -> Result
 
 const FIXED_CORRUPT_MSC4133_FIELDS_MARKER: &[u8] = b"fix_corrupt_msc4133_fields";
 async fn fix_corrupt_msc4133_fields(services: &Services) -> Result {
+	// Due to an old bug, some conduwuit databases have `us.cloke.msc4175.tz` user
+	// profile fields with raw strings instead of quoted JSON ones.
+	// This migration fixes that.
+
 	use serde_json::{Value, from_slice};
 	type KeyVal<'a> = ((OwnedUserId, String), &'a [u8]);
 
@@ -606,24 +610,28 @@ async fn fix_corrupt_msc4133_fields(services: &Services) -> Result {
 			async |(mut total, mut fixed),
 			       ((user, key), value): KeyVal<'_>|
 			       -> Result<(usize, usize)> {
-				if let Err(error) = from_slice::<Value>(value) {
-					// Due to an old bug, some conduwuit databases have `us.cloke.msc4175.tz` user
-					// profile fields with raw strings instead of quoted JSON ones.
-					// This migration fixes that.
-					let new_value = if key == "us.cloke.msc4175.tz" {
-						Value::String(String::from_utf8(value.to_vec())?)
-					} else {
-						return Err!(
-							"failed to deserialize msc4133 key {} of user {}: {}",
-							key,
-							user,
-							error
+				match from_slice::<Value>(value) {
+					// corrupted timezone field
+					| Err(_) if key == "us.cloke.msc4175.tz" => {
+						let new_value = Value::String(String::from_utf8(value.to_vec())?);
+						useridprofilekey_value.put((user, key), Json(new_value));
+						fixed = fixed.saturating_add(1);
+					},
+					// corrupted value for some other key
+					| Err(error) => {
+						warn!(
+							"deleting MSC4133 key {} for user {} due to deserialization \
+							 failure: {}",
+							key, user, error
 						);
-					};
-
-					useridprofilekey_value.put((user, key), Json(new_value));
-					fixed = fixed.saturating_add(1);
+						useridprofilekey_value.del((user, key));
+					},
+					// other key with no issues
+					| Ok(_) => {
+						// do nothing
+					},
 				}
+
 				total = total.saturating_add(1);
 
 				Ok((total, fixed))