Skip to content

fix: 为实例配置反序列化失败补充诊断日志 - #257

Open
Craun718 wants to merge 7 commits into
MaaXYZ:masterfrom
Craun718:codex/fix-instance-config-deserialization-logging
Open

Craun718 wants to merge 7 commits into
MaaXYZ:masterfrom
Craun718:codex/fix-instance-config-deserialization-logging

Conversation

@Craun718

@Craun718 Craun718 commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

实例配置中的历史 ADB 设备等对象损坏或包含无法读取的枚举值时,InstanceConfiguration.TryGetValue 原先会静默吞掉反序列化异常;界面可能显示空白,日志无法区分配置缺失和配置损坏。

将本地反序列化集中到 TryGetLocalValue,并让 TryGetValue 复用它。读取失败会记录实例 ID、配置项、目标类型、配置文件路径及清理后的异常摘要;同一实例、同一配置项及同一坏值内容只报告一次。去重状态在同一进程内跨实例配置对象共享;不同坏值仍可单独诊断。TryGetValue 随后继续现有兼容配置回退流程;TryGetLocalValue 返回 false。缺少配置项时不产生错误日志,调用方仍可继续运行。

此 PR 独立基于 master,对应 #256 中关于损坏历史配置缺少诊断的审查意见。TryGetLocalValue 与 #256 的接口签名一致;整合时需合并重复的方法定义,保留本 PR 带诊断的实现。

验证:git diff --check 通过;dotnet build MFAAvalonia/MFAAvalonia.csproj --no-restore --disable-build-servers -m:1 -v minimal 通过(0 errors,12 warnings)。未进行 GUI 或损坏配置的运行时手动验证。

Sourcery 生成的摘要

为实例配置读取增加可去重的反序列化失败诊断,并保持缺失配置和兼容回退流程的现有行为。

Bug 修复:

  • 为实例配置反序列化失败增加诊断日志,包含实例、配置项、目标类型、文件路径和异常摘要。
  • 避免损坏配置读取异常被静默吞掉,同时保留现有兼容配置回退行为。

增强:

  • 新增仅读取当前实例配置的接口,并对相同实例、配置项及坏值内容导致的重复错误进行进程内去重。
Original summary in English

Sourcery 摘要

为实例配置读取增加可去重的反序列化失败诊断,同时保持缺失配置和兼容回退行为不变。

Bug 修复:

  • 记录实例配置反序列化失败的诊断信息,避免损坏配置读取异常被静默忽略。
  • 保留配置缺失时的正常行为及现有兼容配置回退流程。

功能增强:

  • 新增仅读取当前实例配置的接口,并对同一实例、配置项及坏值内容的重复诊断进行进程内去重。
Original summary in English

Sourcery 摘要

改进实例配置的读取,在保留现有回退行为的同时,诊断损坏的配置值。

Bug 修复:

  • 为实例配置反序列化失败添加诊断日志,避免静默忽略损坏的配置值。
  • 保留缺失值的处理行为,以及本地配置读取失败时现有的兼容性回退机制。

增强功能:

  • 添加仅查询本地实例配置的 API,并在进程范围内对同一实例、键和无效值的重复诊断进行去重。
Original summary in English

Sourcery 总结

为实例配置读取添加可去重且经过脱敏的反序列化失败诊断,同时保持现有的配置回退行为。

Bug 修复:

  • 记录实例配置反序列化失败的诊断信息,避免损坏的配置读取异常被静默忽略。
  • 保留配置缺失时的正常行为,以及现有的兼容配置回退流程。

功能增强:

  • 新增仅读取当前实例配置的接口,并在进程内对同一实例、配置项和错误值内容的重复诊断进行去重。
  • 对诊断日志中的异常摘要和字段进行脱敏,并限制其长度。
Original summary in English

Sourcery 总结

为实例配置读取新增安全且可去重的反序列化失败诊断。

缺陷修复:

  • 记录实例配置反序列化失败的诊断信息,避免损坏的配置被静默忽略,同时保留缺失配置和兼容回退行为。

改进:

  • 新增仅读取当前实例配置的接口,并在进程内对重复的损坏值诊断进行去重。
  • 加强诊断日志的敏感信息脱敏、字段清理和长度限制,并提升日志初始化和写入的并发安全性。
Original summary in English

Summary by Sourcery

为实例配置读取增加安全且可去重的反序列化失败诊断。

Bug Fixes:

  • 记录实例配置反序列化失败的诊断信息,避免损坏配置被静默忽略,同时保留缺失配置和兼容回退行为。

Enhancements:

  • 新增仅读取当前实例配置的接口,并对重复的损坏值诊断进行进程内去重。
  • 增强诊断日志的敏感信息脱敏、字段清理和长度限制,并提升日志初始化与写入的并发安全性。
Original summary in English

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

您好——我发现了 1 个问题

给 AI 代理的提示
请处理本次代码审查中的评论:

## 单条评论

### 评论 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="343-345" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                LoggerHelper.Error(
+                    $"实例配置反序列化失败:实例={_instanceId},配置项={key},目标类型={typeof(T).FullName},配置文件={GetConfigFilePath()}",
+                    ex);
+            }
+        }
</code_context>
<issue_to_address>
**重复失败会导致诊断日志轮转**

重复读取已持久化的格式错误值时(例如反复读取设备设置),`TryGetLocalValue` 会在每次读取时记录相同的反序列化异常,并将格式错误的值留在 `_config` 中。因此,反复读取损坏的设置会持续写入错误日志,并导致较早的诊断日志被轮转清除。

请针对特定实例和键对报告进行去重;或者在首次报告后标记/移除格式错误的值,同时保留预期的回退行为。
</issue_to_address>

Sourcery 对开源项目免费——如果您喜欢我们的审查,请考虑分享 ✨
Original comment in English

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="343-345" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                LoggerHelper.Error(
+                    $"实例配置反序列化失败:实例={_instanceId},配置项={key},目标类型={typeof(T).FullName},配置文件={GetConfigFilePath()}",
+                    ex);
+            }
+        }
</code_context>
<issue_to_address>
**Repeated failures rotate diagnostic logs**

When a persisted malformed value is read repeatedly, such as during repeated device-setting reads, `TryGetLocalValue` logs the same deserialization exception on every read and leaves the malformed value in `_config`, so repeated reads of a damaged setting keep writing errors and rotate out older diagnostic logs.

Deduplicate reports for a given instance and key, or mark/remove the malformed value after the first report while preserving the intended fallback behavior.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
@Craun718

Copy link
Copy Markdown
Member Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你好——我发现了 3 个问题

给 AI 智能体的提示
请处理此代码审查中的评论:

## 各条评论

### 评论 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="350-351" />
<code_context>
+                if (_reportedDeserializationFailures.TryAdd(key, 0))
+                {
+                    LoggerHelper.Error(
+                        $"实例配置反序列化失败:实例={_instanceId},配置项={key},目标类型={typeof(T).FullName},配置文件={GetConfigFilePath()}",
+                        ex);
+                }
+            }
</code_context>
<issue_to_address>
**格式错误的值伪造日志条目**

攻击者提供修改过的实例配置时,如果无效值出现在反序列化异常中且包含换行符,`LoggerHelper.Error` 会写入该反序列化异常,而异常消息可能包含格式错误的配置值;Serilog 输出异常时不会转义其中的换行符,因此精心构造的值会生成误导性的额外日志行。

在记录日志前清理或结构化异常详细信息,以防不受信任的值生成额外日志条目。
</issue_to_address>

### 评论 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="347" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                if (_reportedDeserializationFailures.TryAdd(key, 0))
+                {
+                    LoggerHelper.Error(
</code_context>
<issue_to_address>
**更新后的失败未被报告**

当某个键的反序列化读取与 `SetValue` 或 `ReloadFromDisk` 同时发生,并在其失败标记被清除后才完成时,`TryGetLocalValue` 会让过期读取中的 `_reportedDeserializationFailures.TryAdd` 重新设置该标记,从而抑制对更新后值的后续诊断信息。

防止正在进行的读取为已因写入或重新加载而失效的值添加失败标记。

另请参阅 `MFAAvalonia/Configuration/InstanceConfiguration.cs:98`、`MFAAvalonia/Configuration/InstanceConfiguration.cs:397`。
</issue_to_address>

### 评论 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="16" />
<code_context>
 {
     private readonly string _instanceId;
     private Dictionary<string, object> _config;
+    private readonly ConcurrentDictionary<string, byte> _reportedDeserializationFailures = new();
     private volatile bool _isDeleted;

</code_context>
<issue_to_address>
**损坏的键会被反复记录**

当同一 ID 对应的多个 `InstanceConfiguration` 对象读取同一个损坏的键时,由于 `_reportedDeserializationFailures` 是实例级的,`TryAdd` 会在每个对象中都成功,导致针对该实例反复记录同一失败。

在同一实例 ID 对应的 `InstanceConfiguration` 对象之间共享已报告失败的状态。

另请参阅 `MFAAvalonia/Configuration/InstanceConfiguration.cs:347-352`。
</issue_to_address>

Sourcery 为开源项目免费提供服务——如果您喜欢我们的审查,欢迎分享 ✨
Original comment in English

Hey - I've found 3 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="350-351" />
<code_context>
+                if (_reportedDeserializationFailures.TryAdd(key, 0))
+                {
+                    LoggerHelper.Error(
+                        $"实例配置反序列化失败:实例={_instanceId},配置项={key},目标类型={typeof(T).FullName},配置文件={GetConfigFilePath()}",
+                        ex);
+                }
+            }
</code_context>
<issue_to_address>
**Malformed values forge log entries**

When an attacker supplies a modified instance configuration whose invalid value is reflected in a deserialization exception and includes line breaks, `LoggerHelper.Error` writes the deserialization exception, whose message can include the malformed configuration value; Serilog renders exceptions without escaping embedded newlines, so a crafted value creates misleading additional log lines.

Sanitize or structure exception details before logging so untrusted values cannot create additional log entries.
</issue_to_address>

### Comment 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="347" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                if (_reportedDeserializationFailures.TryAdd(key, 0))
+                {
+                    LoggerHelper.Error(
</code_context>
<issue_to_address>
**Updated failures go unreported**

When a deserialization read for a key overlaps `SetValue` or `ReloadFromDisk` and finishes after its failure marker is cleared, `TryGetLocalValue` then lets the stale read's `_reportedDeserializationFailures.TryAdd` restore the marker, suppressing the updated value's later diagnostic.

Prevent in-flight reads from adding failure markers for values invalidated by a write or reload.

Also at `MFAAvalonia/Configuration/InstanceConfiguration.cs:98`, `MFAAvalonia/Configuration/InstanceConfiguration.cs:397`.
</issue_to_address>

### Comment 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="16" />
<code_context>
 {
     private readonly string _instanceId;
     private Dictionary<string, object> _config;
+    private readonly ConcurrentDictionary<string, byte> _reportedDeserializationFailures = new();
     private volatile bool _isDeleted;

</code_context>
<issue_to_address>
**Corrupt keys log repeatedly**

When multiple `InstanceConfiguration` objects for the same ID read the same corrupt key, because `_reportedDeserializationFailures` is instance-local, `TryAdd` succeeds in each object and logs the same failure repeatedly for that instance.

Share the reported-failure state across `InstanceConfiguration` objects for the same instance ID.

Also at `MFAAvalonia/Configuration/InstanceConfiguration.cs:347-352`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
@Craun718

Copy link
Copy Markdown
Member Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗨——我发现了 4 个问题

供 AI Agent 使用的提示
请处理此代码审查中的评论:

## 单条评论

### 评论 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="354" />
<code_context>
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)},异常摘要={SanitizeLogField(ex.Message)}");
+                }
+            }
</code_context>
<issue_to_address>
**配置值泄露到日志中**

当转换器拒绝包含敏感文本的配置值时,记录的异常摘要会包含 `ex.Message`;转换器可能会直接将被拒绝的值放入该消息中,而 `SanitizeLogField` 只会移除控制字符。因此,包含敏感文本的格式错误配置值会被写入应用程序日志。

请记录长度受限且经过脱敏的异常摘要,排除原始的被拒绝值以及其他可能包含敏感信息的消息内容。
</issue_to_address>

### 评论 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="18-19" />
<code_context>
 {
     private readonly string _instanceId;
     private Dictionary<string, object> _config;
+    private static readonly ConcurrentDictionary<(string InstanceId, string Key, string ContentHash), byte>
+        ReportedDeserializationFailures = new();
     private volatile bool _isDeleted;

</code_context>
<issue_to_address>
**失败跟踪数据会无限增长**

长期运行的进程读取大量不同的无效值时,`ReportedDeserializationFailures` 会为每个不同的失败值保留一条记录,且没有清理机制,因此进程运行期间内存用量会持续增长。

请限制失败去重存储的大小,或淘汰其中的条目。

另请查看 `MFAAvalonia/Configuration/InstanceConfiguration.cs:352`。
</issue_to_address>

### 评论 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="350" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                var failureContent = serializedData ?? $"{ex.GetType().FullName}\0{ex.Message}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
</code_context>
<issue_to_address>
**不同的序列化失败会被忽略**

如果为同一实例和键序列化不同的值时抛出相同类型且消息相同的异常,那么 `failureContent` 只会包含异常类型和消息,因此 `contentHash` 会匹配,`ReportedDeserializationFailures.TryAdd` 就会抑制后续诊断;调用方只能看到第一次失败。

请使用一种指纹,即使序列化抛出异常,也能区分失败的值。
</issue_to_address>

### 评论 4
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="352" />
<code_context>
+            {
+                var failureContent = serializedData ?? $"{ex.GetType().FullName}\0{ex.Message}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)},异常摘要={SanitizeLogField(ex.Message)}");
</code_context>
<issue_to_address>
**转换失败未被记录**

同一个本地值针对不同目标类型或不同转换器集转换失败时,`ReportedDeserializationFailures` 的键只包含实例、键和序列化内容,因此后续的 `TryGetLocalValue<T>` 调用会被抑制,其特定于类型的失败也就不会写入日志。

请将目标类型和转换器集纳入去重键。
</issue_to_address>

Sourcery 对开源项目免费——如果您喜欢我们的审查,请考虑分享 ✨
Original comment in English

Hey - I've found 4 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="354" />
<code_context>
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)},异常摘要={SanitizeLogField(ex.Message)}");
+                }
+            }
</code_context>
<issue_to_address>
**Configuration values leak into logs**

When a converter rejects a configuration value containing sensitive text, the logged exception summary includes `ex.Message`; converters can put the rejected value directly in that message, and `SanitizeLogField` only removes control characters. A malformed configuration value that contains sensitive text is therefore written to the application log.

Log a bounded, redacted exception summary that excludes raw rejected values and other potentially sensitive message content.
</issue_to_address>

### Comment 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="18-19" />
<code_context>
 {
     private readonly string _instanceId;
     private Dictionary<string, object> _config;
+    private static readonly ConcurrentDictionary<(string InstanceId, string Key, string ContentHash), byte>
+        ReportedDeserializationFailures = new();
     private volatile bool _isDeleted;

</code_context>
<issue_to_address>
**Failure tracking grows without bound**

When a long-running process reads many distinct invalid values, `ReportedDeserializationFailures` retains an entry for each distinct failing value, with no removal path, so memory usage keeps growing for the life of the process.

Bound or evict entries from the failure-deduplication store.

Also at `MFAAvalonia/Configuration/InstanceConfiguration.cs:352`.
</issue_to_address>

### Comment 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="350" />
<code_context>
+            }
+            catch (Exception ex)
+            {
+                var failureContent = serializedData ?? $"{ex.GetType().FullName}\0{ex.Message}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
</code_context>
<issue_to_address>
**Distinct serialization failures disappear**

When serializing different values for the same instance and key throws the same exception type and message, `failureContent` then contains only the exception type and message, so `contentHash` matches and `ReportedDeserializationFailures.TryAdd` suppresses the later diagnostic; callers see only the first failure.

Use a fingerprint that distinguishes the failing values even when serialization throws.
</issue_to_address>

### Comment 4
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="352" />
<code_context>
+            {
+                var failureContent = serializedData ?? $"{ex.GetType().FullName}\0{ex.Message}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                if (ReportedDeserializationFailures.TryAdd((_instanceId, key, contentHash), 0))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)},异常摘要={SanitizeLogField(ex.Message)}");
</code_context>
<issue_to_address>
**Conversion failures go unreported**

When the same local value fails conversion for different target types or converter sets, `ReportedDeserializationFailures` keys only on instance, key, and serialized content, so the later `TryGetLocalValue<T>` call is suppressed and its type-specific failure never reaches the log.

Include the target type and converter set in the deduplication key.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
@Craun718

Copy link
Copy Markdown
Member Author

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗨——我发现了 3 个问题

给 AI 代理的提示
请处理这次代码审查中的评论:

## 单条评论

### 评论 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="364" />
<code_context>
+                    _instanceId, key, contentHash, targetType, converterSignature);
+                if (TryRegisterDeserializationFailure(fingerprint))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)}");
+                }
+            }
</code_context>
<issue_to_address>
**并发诊断会破坏日志缓存**

如果多个线程在 `LoggerHelper.InitializeLogger` 完成之前读取了不同的无效值,不同的损坏配置键可能会在各个线程上注册不同的指纹,并并发调用 `LoggerHelper.Error`。日志记录器初始化之前,`LoggerHelper.Write` 会向共享且未进行同步的 `_logCache` 列表追加内容,因此诊断信息可能丢失,或者调用方在读取配置时抛出异常。

请确保初始化前对日志缓存的写入和刷新是线程安全的,或者通过线程安全的队列串行处理诊断日志。
</issue_to_address>

### 评论 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="364" />
<code_context>
+                    _instanceId, key, contentHash, targetType, converterSignature);
+                if (TryRegisterDeserializationFailure(fingerprint))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)}");
+                }
+            }
</code_context>
<issue_to_address>
**反序列化原因被隐藏**

如果不同的本地转换失败具有相同的异常类型,`TryGetLocalValue` 只会记录异常类型,因此无效的枚举值和具有该异常类型的转换器故障会产生无法区分的诊断信息,运维人员也就无法判断具体原因。

请在记录的诊断信息中包含异常消息或异常对象。

`MFAAvalonia/Configuration/InstanceConfiguration.cs:365` 处也有此问题。
</issue_to_address>

### 评论 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="357-358" />
<code_context>
+                        ? "<null>"
+                        : converter.GetType().AssemblyQualifiedName ?? converter.GetType().FullName ?? converter.GetType().Name));
+                var targetType = typeof(T).AssemblyQualifiedName ?? typeof(T).FullName ?? typeof(T).Name;
+                var failureContent = serializedData ??
+                    $"serialization-failure\0{(data is null ? 0 : RuntimeHelpers.GetHashCode(data))}\0{ex.GetType().AssemblyQualifiedName}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                var fingerprint = new DeserializationFailureFingerprint(
</code_context>
<issue_to_address>
**序列化失败的去重逻辑不正确**

如果在不同的配置对象中对等价的值进行序列化时失败,或者同一对象中的值后来发生了变化,`failureContent` 使用的是 `RuntimeHelpers.GetHashCode(data)`,而不是值的内容。因此,等价的值会得到不同的指纹;而发生变化的值可能仍保留相同的指纹,导致后续诊断被抑制。

请使用失败值的稳定表示来构建序列化失败指纹,而不是使用对象标识。
</issue_to_address>

Sourcery 对开源项目免费——如果您喜欢我们的审查结果,欢迎分享 ✨
Original comment in English

Hey - I've found 3 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="364" />
<code_context>
+                    _instanceId, key, contentHash, targetType, converterSignature);
+                if (TryRegisterDeserializationFailure(fingerprint))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)}");
+                }
+            }
</code_context>
<issue_to_address>
**Concurrent diagnostics corrupt log cache**

When multiple threads read different invalid values before `LoggerHelper.InitializeLogger` completes, different corrupt keys can register distinct fingerprints on separate threads and call `LoggerHelper.Error` concurrently. Before the logger is initialized, `LoggerHelper.Write` appends to the shared, unsynchronized `_logCache` list, so diagnostics are lost or a caller throws while reading configuration.

Make pre-initialization log-cache writes and flushing thread-safe, or serialize diagnostic logging through a thread-safe queue.
</issue_to_address>

### Comment 2
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="364" />
<code_context>
+                    _instanceId, key, contentHash, targetType, converterSignature);
+                if (TryRegisterDeserializationFailure(fingerprint))
+                {
+                    LoggerHelper.Error($"实例配置反序列化失败:实例={SanitizeLogField(_instanceId)},配置项={SanitizeLogField(key)},目标类型={SanitizeLogField(typeof(T).FullName ?? typeof(T).Name)},配置文件={SanitizeLogField(GetConfigFilePath())},异常类型={SanitizeLogField(ex.GetType().FullName ?? ex.GetType().Name)}");
+                }
+            }
</code_context>
<issue_to_address>
**Deserialization cause is hidden**

When different local conversion failures have the same exception type, `TryGetLocalValue` logs only the exception type, so invalid enum values and converter failures with that type produce indistinguishable diagnostics and operators cannot identify the cause.

Include the exception message or exception object in the logged diagnostic.

Also at `MFAAvalonia/Configuration/InstanceConfiguration.cs:365`.
</issue_to_address>

### Comment 3
<location path="MFAAvalonia/Configuration/InstanceConfiguration.cs" line_range="357-358" />
<code_context>
+                        ? "<null>"
+                        : converter.GetType().AssemblyQualifiedName ?? converter.GetType().FullName ?? converter.GetType().Name));
+                var targetType = typeof(T).AssemblyQualifiedName ?? typeof(T).FullName ?? typeof(T).Name;
+                var failureContent = serializedData ??
+                    $"serialization-failure\0{(data is null ? 0 : RuntimeHelpers.GetHashCode(data))}\0{ex.GetType().AssemblyQualifiedName}";
+                var contentHash = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(failureContent)));
+                var fingerprint = new DeserializationFailureFingerprint(
</code_context>
<issue_to_address>
**Serialization failures deduplicate incorrectly**

When serialization fails for equivalent values in separate configuration objects or a value is later mutated on the same object, `failureContent` uses `RuntimeHelpers.GetHashCode(data)` instead of value content, so equivalent values get different fingerprints while mutations can retain the same fingerprint and suppress a later diagnostic.

Build the serialization-failure fingerprint from a stable representation of the failing value rather than its object identity.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Comment thread MFAAvalonia/Configuration/InstanceConfiguration.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant