Release 0.9.2 #17
Release 0.9.2 #17
Conversation
….LogError, to properly show the issues in the Unity console
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe changes in this pull request involve updates to the error handling mechanisms across several classes in the state management system, enhancing logging capabilities instead of throwing exceptions. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StateInternal
participant TaskWaitState
participant WaitState
User->>StateInternal: TriggerEnter()
StateInternal->>StateInternal: Log Error
StateInternal->>User: Return
User->>TaskWaitState: InnerTaskAwait()
TaskWaitState->>TaskWaitState: Log Error
TaskWaitState->>User: Return
User->>WaitState: InnerWait()
WaitState->>WaitState: Log Error
WaitState->>User: Return
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
WalkthroughThe changes made to the codebase involve updating exception handling by replacing try/catch blocks with Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (4)
- CHANGELOG.md (1)
- Runtime/Internal/StateInternal.cs (3)
- Runtime/Internal/TaskWaitState.cs (1)
- Runtime/Internal/WaitState.cs (5)
Files ignored due to filter (1)
- package.json
Files skipped from review due to trivial changes (1)
- Runtime/Internal/WaitState.cs
Review comments generated (4)
- Review: 4
- LGTM: 0
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| var finalMessage = ""; | ||
| #if UNITY_EDITOR || DEBUG | ||
| finalMessage = $"\nStackTrace log of '{state.Name}' state creation bellow.\n{CreationStackTrace}"; | ||
| #endif | ||
|
|
||
| Debug.LogError($"Exception in the state '{state.Name}', OnExit() actions.\n" + | ||
| $"-->> Check the exception log after this one for more details <<-- {finalMessage}"); | ||
| Debug.LogException(e); |
There was a problem hiding this comment.
The try/catch block has been modified to log the exception using Debug.LogError and Debug.LogException. The code also includes a conditional message for logging exceptions in the Unity Editor or Debug mode. This change improves error visibility in the Unity console.
However, there are a few suggestions to consider:
-
Line 168: The variable
finalMessageis initialized as an empty string and then assigned a value based on the conditionUNITY_EDITOR || DEBUG. Instead of initializing it as an empty string, you can directly assign the value inside the condition. This will eliminate the need for an extra line of code. -
Line 170: The comment mentions "StackTrace log of '{state.Name}' state creation below," but the code actually logs the
CreationStackTraceinstead of the state name. Consider updating the comment to accurately reflect the logged information. -
Line 172: The error message includes the text "-->> Check the exception log after this one for more details <<--". It's unclear what "exception log" refers to. Consider providing more specific instructions or clarifying the intended behavior.
-
Line 173: The error message includes the state name, but it's not clear why the state name is relevant in the context of the exception. Consider removing the state name from the error message unless it provides valuable information for debugging.
-
Line 174: The
Debug.LogException(e)statement logs the exception object. It's important to note that logging the entire exception object may expose sensitive information in a production environment. Ensure that the exception object does not contain any sensitive data before logging it.
| var finalMessage = ""; | ||
| #if UNITY_EDITOR || DEBUG | ||
| finalMessage = $"\nStackTrace log of '{Name}' state creation bellow.\n{CreationStackTrace}"; | ||
| #endif | ||
|
|
||
| Debug.LogError($"Exception in the state '{Name}', OnExit() actions.\n" + | ||
| $"-->> Check the exception log after this one for more details <<-- {finalMessage}"); | ||
| Debug.LogException(e); |
There was a problem hiding this comment.
Similar changes have been made to another catch block in the same file. The suggestions mentioned above apply to this section as well.
| var finalMessage = ""; | ||
| #if UNITY_EDITOR || DEBUG | ||
| finalMessage = $"\nStackTrace log of this transition creation bellow.\n{transition.CreationStackTrace}"; | ||
| #endif | ||
|
|
||
| Debug.LogError($"Exception in the transition '{Name}' -> '{transition.TargetState?.Name}'" + | ||
| $"-->> Check the exception log after this one for more details <<-- {finalMessage}"); | ||
| Debug.LogException(e); |
There was a problem hiding this comment.
Another catch block has been modified to log the exception using Debug.LogError and Debug.LogException. The code includes a conditional message for logging exceptions in the Unity Editor or Debug mode. The suggestions mentioned above also apply to this section.
Overall, the changes improve error visibility in the Unity console. However, it's important to review the error messages and ensure they provide clear and relevant information for debugging purposes. Additionally, be cautious about logging sensitive information in the exception object.
| var finalMessage = ""; | ||
| #if UNITY_EDITOR || DEBUG | ||
| finalMessage = $"\nStackTrace log of '{Name}' state creation bellow.\n{CreationStackTrace}"; | ||
| #endif | ||
|
|
||
| Debug.LogError($"Exception in the state '{Name}', when calling the task wait action" + | ||
| $"'{_taskAwaitAction.Target}.{_taskAwaitAction.Method.Name}()'.\n" + | ||
| $"-->> Check the exception log after this one for more details <<-- {finalMessage}"); | ||
| Debug.LogException(e); |
There was a problem hiding this comment.
The exception handling in this code has been modified. Instead of throwing a new exception with a detailed message and stack trace, the code now logs an error message using Debug.LogError and logs the exception itself using Debug.LogException. Additionally, a conditional message is added to include the creation stack trace when running in the Unity Editor or Debug mode.
While the change improves error visibility in the Unity console, it removes the ability to catch and handle the exception at a higher level. This may limit the flexibility of error handling and make it harder to debug issues that occur during the task wait action.
Consider the following suggestions:
-
If catching and handling the exception at a higher level is necessary, you can revert the changes and keep the original code that throws a new exception with a detailed message and stack trace. This allows for more flexibility in error handling.
-
If logging the exception and its stack trace is sufficient for debugging purposes, you can keep the current implementation. However, it would be beneficial to include the exception type in the log message to provide more context about the error. For example:
Debug.LogError($"Exception of type {e.GetType()} occurred in the state '{Name}', when calling the task wait action " +
$"'{_taskAwaitAction.Target}.{_taskAwaitAction.Method.Name}()'.\n" +
$"-->> Check the exception log after this one for more details <<-- {finalMessage}");- If you want to strike a balance between logging the exception and allowing for higher-level error handling, you can modify the code to rethrow the exception after logging it. This way, the exception will still propagate up the call stack, but you'll have a record of it in the log. Here's an example:
Debug.LogError($"Exception in the state '{Name}', when calling the task wait action " +
$"'{_taskAwaitAction.Target}.{_taskAwaitAction.Method.Name}()'.\n" +
$"-->> Check the exception log after this one for more details <<-- {finalMessage}");
Debug.LogException(e);
throw;Choose the approach that best suits your error handling and debugging requirements.
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes for Version 0.9.2
New Features
Bug Fixes
Documentation
Chores