Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| Asset Size: {asset.S} | ||
| Asset Hash: {asset.CRC} | ||
| Type: {asset.FT} | ||
| Object Association: {(asset.AssociatedObject is var obj ? obj.GetType().Name : "GenericDownload")} |
There was a problem hiding this comment.
Bug: The pattern asset.AssociatedObject is var obj can assign null to obj. A subsequent call to obj.GetType().Name will throw a NullReferenceException, masking the original exception.
Severity: MEDIUM
Suggested Fix
Add a null check for obj after the pattern match asset.AssociatedObject is var obj. If obj is null, handle it as a generic asset case instead of attempting to access obj.GetType(). For example: _ => asset.AssociatedObject is var obj && obj != null ? $"Repairing asset: {obj.GetType().Name}" : "GenericDownload".
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
CollapseLauncher/Classes/RepairManagement/HonkaiV2/HonkaiRepairV2.Repair.cs#L87
Potential issue: Inside an `HttpRequestException` catch block, the code uses
`asset.AssociatedObject is var obj` to pattern match. This expression always evaluates
to true, even if `asset.AssociatedObject` is `null`. For generic assets, this property
is often `null`, causing `obj` to be `null`. The following line, `obj.GetType().Name`,
then throws a `NullReferenceException`. This new exception prevents the original network
exception from being handled correctly and makes the `"GenericDownload"` fallback branch
unreachable.
Did we get this right? 👍 / 👎 to inform future reviews.
|
Waiting for further internal test before releasing. |
| UrlStatus urlStatus = await assetBundleHttpClient.GetURLStatusCode(assetUrl, innerToken); | ||
| Logger.LogWriteLine($"The CG asset: {assetName} " + | ||
| (urlStatus.IsSuccessStatusCode ? "is" : "is not") + $" available (Status code: {urlStatus.StatusCode})", LogType.Default, true); | ||
| if (!urlStatus.IsSuccessStatusCode) | ||
| { | ||
| UrlStatus urlStatus = await assetBundleHttpClient.GetURLStatusCode(assetUrl, innerToken); | ||
| Logger.LogWriteLine($"The CG asset: {assetName} " + | ||
| (urlStatus.IsSuccessStatusCode ? "is" : "is not") + $" available (Status code: {urlStatus.StatusCode})", LogType.Default, true); | ||
| if (!urlStatus.IsSuccessStatusCode) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (urlStatus.FileSize > 0) | ||
| { | ||
| assetFilesize = urlStatus.FileSize; | ||
| } | ||
| throw new HttpRequestException("No Asset bundle URLs were reachable"); |
There was a problem hiding this comment.
Bug: Throwing an HttpRequestException for missing optional assets inside a Parallel.ForEachAsync loop terminates the entire repair process, instead of gracefully skipping the asset.
Severity: HIGH
Suggested Fix
Instead of throwing an exception when a non-success status code is received for an optional asset, the code should log the event and continue processing other assets. This can be achieved by removing the throw new HttpRequestException(...) and replacing it with logic to skip the current asset, similar to the previous behavior.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
CollapseLauncher/Classes/RepairManagement/HonkaiV2/HonkaiRepairV2.AsbExt.Video.cs#L138-L143
Potential issue: In the game repair process for Honkai Impact 3rd, fetching optional
video or audio assets can result in a non-successful HTTP status like a 404, which is an
expected scenario for seasonal or unavailable content. The updated code incorrectly
handles this by throwing an `HttpRequestException`. This exception is thrown within a
`Parallel.ForEachAsync` loop, which causes the entire parallel asset fetch operation to
terminate prematurely. As a result, the entire repair process fails, whereas the
previous implementation would have gracefully skipped the missing asset and continued.
Also affects:
CollapseLauncher/Classes/RepairManagement/HonkaiV2/HonkaiRepairV2.AsbExt.Audio.cs:107~110
| // Get gatewayURl and fetch the gateway | ||
| GameGateway = | ||
| await KianaDispatch.GetGameserver(client, dispatch!, GameVersionManager.GamePreset.GameGatewayDefault!, token); | ||
| GameGateway = await KianaDispatch.GetGameserver(client, dispatch!, GameVersionManager.GamePreset.GameGatewayDefault!, token); |
There was a problem hiding this comment.
Bug: A NullReferenceException will occur when calling GetGameserver with dispatch! because dispatch can remain null if GameDispatchURLTemplate or GameDispatchChannelName is null.
Severity: CRITICAL
Suggested Fix
Ensure the dispatch variable is handled correctly when it is null. Either throw a more informative exception immediately after the conditional check if dispatch is null, or adjust the logic to prevent the call to KianaDispatch.GetGameserver from executing with a null dispatch variable.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: CollapseLauncher/Classes/CachesManagement/Honkai/Fetch.cs#L127
Potential issue: The `dispatch` variable is initialized within a conditional block that
checks if `GameDispatchURLTemplate` and `GameDispatchChannelName` are non-null. If
either of these properties is null, the condition is false, and `dispatch` remains
uninitialized (i.e., null). However, the code later calls `KianaDispatch.GetGameserver`
using `dispatch!` with the null-forgiving operator. This will cause a
`NullReferenceException` at runtime if the initial condition was not met, crashing the
application during the cache fetching process.
1 participant
Preview 1.84.3 - Emergency Hotfix (Codename: Columbina)
What's changed?
HttpRequestException, by @neon-nyanstreamingasb:80URL while downloading in some cases, by @neon-nyanKnown Issues
InvalidOperationException: An invalid request URI was provided. Either the request URI must be an absolute URI or BaseAddress must be set.error while performing Game RepairFull Changelog: CL-v1.84.2-pre...CL-v1.84.3-pre
Code Signing Policy