Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-4965] Implement logging #39

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Sep 5, 2024

The aim of this commit is to:

  • provide infrastructure for internal components to be able to log messages
  • add some log messages to the existing functionality (I am not particularly concerned by the quality or utility of these log messages at the moment)
  • provide a basic default logging backend so that users can see log messages in the console

The choice to have a separate internal logging interface is taken from Asset Tracking (AAT) and ably-cocoa, where I think it works well. I haven’t currently copied across AAT’s subsystem mechanism, a large part of the motivation for which was to provide a way to integrate the logs emitted by the AAT-instantiated Realtime client into those emitted by AAT itself; this is not currently a goal for Chat, which does not instantiate a Realtime client.

I briefly toyed with the idea of writing a macro that would log the inputs and outputs of public API methods, but figuring out how to do that didn’t seem like a good use of time now.

Resolves #38.

Summary by CodeRabbit

  • New Features

    • Enhanced logging capabilities across the chat client, rooms, and room status, improving error tracking and debugging.
    • Introduced a new InternalLogger protocol for better logging context and control.
  • Bug Fixes

    • Improved error handling during channel attachment and detachment in the chat room.
  • Tests

    • Added comprehensive unit tests for the new logging functionalities, ensuring expected behavior of the logging system.
    • Updated existing tests to incorporate the new logging parameters for better validation.

Copy link

coderabbitai bot commented Sep 5, 2024

Walkthrough

The changes introduce a comprehensive logging system to the AblyChat module, enhancing the ability to track and manage log messages throughout the application. This includes the implementation of an InternalLogger protocol, various logger classes, and the integration of logging capabilities into key components like chat clients and rooms. The modifications ensure that logging is consistent and provides contextual information for improved debugging.

Changes

Files Change Summary
Sources/AblyChat/ChatClient.swift Introduced logger property in DefaultChatClient, modified room initialization to include logger.
Sources/AblyChat/Logging.swift Extended LogLevel to conform to Comparable, added InternalLogger protocol, and implemented DefaultInternalLogger.
Sources/AblyChat/Room.swift Added logger property to DefaultRoom, updated methods to log errors during channel operations.
Sources/AblyChat/RoomStatus.swift Introduced logger property in DefaultRoomStatus, logging added for state transitions.
Sources/AblyChat/Rooms.swift Modified DefaultRooms to accept logger, ensuring room instances can log events.
Tests/AblyChatTests/DefaultInternalLoggerTests.swift Added unit tests for DefaultInternalLogger functionality and defaults.
Tests/AblyChatTests/DefaultRoomStatusTests.swift Updated tests for DefaultRoomStatus to include logger during initialization.
Tests/AblyChatTests/DefaultRoomTests.swift Updated tests for DefaultRoom to include logger during initialization.
Tests/AblyChatTests/DefaultRoomsTests.swift Updated tests for DefaultRooms to include logger during initialization.
Tests/AblyChatTests/Helpers/TestLogger.swift Introduced TestLogger for testing logging functionality without actual logging.
Tests/AblyChatTests/InternalLoggerTests.swift Added SynchronizedAccess for thread-safe access and tests for MockInternalLogger.
Tests/AblyChatTests/Mocks/MockInternalLogger.swift Implemented MockInternalLogger to capture log messages for testing.
Tests/AblyChatTests/Mocks/MockLogHandler.swift Created MockLogHandler to simulate logging behavior in tests.

Assessment against linked issues

Objective Addressed Explanation
Implement logging (add default logger, pass around)
Emit log statements in the codebase

In the meadow, I hop with glee,
Logs are dancing, wild and free!
With every message, clear and bright,
My chat's now shining, what a sight!
Hooray for logs, both near and far,
They guide us well, like a twinkling star!
🐇✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@lawrence-forooghian lawrence-forooghian changed the title Implement logging [ECO-4965] Implement logging Sep 5, 2024
@lawrence-forooghian lawrence-forooghian marked this pull request as ready for review September 5, 2024 19:55
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3b7a54e and ead9e26.

Files selected for processing (13)
  • Sources/AblyChat/ChatClient.swift (1 hunks)
  • Sources/AblyChat/Logging.swift (1 hunks)
  • Sources/AblyChat/Room.swift (2 hunks)
  • Sources/AblyChat/RoomStatus.swift (2 hunks)
  • Sources/AblyChat/Rooms.swift (2 hunks)
  • Tests/AblyChatTests/DefaultInternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomStatusTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomTests.swift (4 hunks)
  • Tests/AblyChatTests/DefaultRoomsTests.swift (3 hunks)
  • Tests/AblyChatTests/Helpers/TestLogger.swift (1 hunks)
  • Tests/AblyChatTests/InternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockInternalLogger.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockLogHandler.swift (1 hunks)
Files skipped from review due to trivial changes (1)
  • Tests/AblyChatTests/Helpers/TestLogger.swift
Additional comments not posted (29)
Tests/AblyChatTests/Mocks/MockLogHandler.swift (2)

3-3: Clarify the use of @SynchronizedAccess and @unchecked Sendable.

Please confirm if @SynchronizedAccess is a custom attribute or part of Swift's concurrency model. Additionally, consider the implications of using @unchecked Sendable for bypassing concurrency checks.


6-8: Mock implementation of log function is correct.

The log function correctly captures the log message, level, and context, which is suitable for a mock implementation.

Tests/AblyChatTests/Mocks/MockInternalLogger.swift (2)

3-3: Clarify the use of @SynchronizedAccess and @unchecked Sendable.

Please confirm if @SynchronizedAccess is a custom attribute or part of Swift's concurrency model. Additionally, consider the implications of using @unchecked Sendable for bypassing concurrency checks.


6-8: Mock implementation of log function is correct.

The log function correctly captures the log message, level, and code location, which is suitable for a mock implementation.

Sources/AblyChat/Rooms.swift (1)

14-14: Logging integration in DefaultRooms is correctly implemented.

The addition of the logger property and its usage in the constructor and get function are correctly implemented. This enhances the logging capabilities of the DefaultRooms actor, aligning with the PR objectives.

Please ensure that the logger is properly integrated and utilized throughout the rest of the codebase.

Also applies to: 19-22, 36-36

Tests/AblyChatTests/InternalLoggerTests.swift (1)

4-31: Thread-safe property wrapper implementation looks good.

The SynchronizedAccess property wrapper uses NSLock to ensure thread-safe access to its wrapped value, which is a robust and standard approach in Swift.

Tests/AblyChatTests/DefaultRoomStatusTests.swift (3)

6-6: Correct integration of logging in test setup.

The addition of the TestLogger instance in the DefaultRoomStatus constructor is well-aligned with the PR objectives to enhance logging capabilities. The test method correctly verifies the initial state of the current property.


12-12: Proper use of logging in test for initial state.

The integration of the TestLogger in the DefaultRoomStatus constructor facilitates the verification of the initial state of the error property and aligns with the PR's focus on enhancing logging capabilities.


19-19: Effective testing of state transitions with logging.

The test_transition method effectively tests the state transition functionality of DefaultRoomStatus, and the use of TestLogger enhances the logging capabilities during the test, which is in line with the PR objectives.

Sources/AblyChat/ChatClient.swift (1)

17-23: Proper integration of logging in DefaultChatClient.

The introduction of the logger property and its initialization using clientOptions enhance the logging capabilities of the chat client. The use of the logger in the initialization of the rooms property ensures that logging is integrated throughout the chat client's operations, aligning with the PR objectives.

Tests/AblyChatTests/DefaultInternalLoggerTests.swift (3)

4-11: Review of test_defaults function: Well-structured and clear test.

This test function checks the default behavior of the DefaultInternalLogger when no specific log handler or log level is provided. It correctly asserts that the default log handler is an instance of DefaultLogHandler and the default log level is .info. The test is concise and covers the expected behavior effectively.


12-29: Review of test_log function: Comprehensive and correctly implemented.

This test function verifies the behavior of the log method in DefaultInternalLogger. It checks that the log message, level, and code location are correctly passed to the underlying log handler. The use of XCTUnwrap to ensure that logArguments is not nil before proceeding with assertions is a good practice, ensuring the test fails cleanly if the preconditions are not met. The interpolation of the code location into the message is correctly verified.


31-48: Review of test_log_whenLogLevelArgumentIsLessSevereThanLogLevelProperty_itDoesNotLog function: Correctly tests log level filtering.

This test function evaluates the logger's behavior when the log level of a message is less severe than the logger's configured log level. It correctly asserts that no logging should occur in this scenario by checking that logArguments remains nil. This test is crucial for ensuring that the log level filtering works as intended, preventing unnecessary log messages from being processed.

Sources/AblyChat/RoomStatus.swift (2)

40-44: Review of logger initialization in DefaultRoomStatus: Properly implemented.

The logger property is correctly declared as private and is initialized in the actor's constructor. This encapsulation ensures that the logger can only be modified internally within the actor, which is a good practice for maintaining control over the logging behavior.


57-57: Review of logging in transition(to:) method: Effective use of logging.

The addition of a logging statement within the transition(to:) method is a good practice. It logs the new state to which the actor is transitioning, providing valuable debugging information. The log level .debug is appropriately chosen for this kind of operational detail, which is typically more verbose and intended for debugging rather than regular operation monitoring.

Tests/AblyChatTests/DefaultRoomsTests.swift (3)

9-9: Review of logger addition in test_get_returnsRoomWithGivenID: Correctly integrated.

The addition of the logger parameter in the instantiation of DefaultRooms within the test setup is correctly implemented. This change allows the test to utilize the enhanced logging capabilities of the DefaultRooms class, potentially aiding in debugging and verifying the behavior of room retrieval based on ID.


27-27: Review of logger addition in test_get_returnsExistingRoomWithGivenID: Consistent with other test setups.

Similar to the previous test, the addition of the logger parameter here is consistent and correctly implemented. It ensures that the logging functionality is available during the test, which can be crucial for tracing and debugging the retrieval of existing rooms.


44-44: Review of logger addition in test_get_throwsErrorWhenOptionsDoNotMatch: Enhances test diagnostics.

The inclusion of the logger parameter in this test setup is particularly useful, as it enhances the diagnostic capabilities when testing error conditions. This is important for understanding the behavior of the DefaultRooms class when room options do not match, and it helps ensure that the error handling is robust and well-documented through logs.

Sources/AblyChat/Room.swift (4)

27-28: Addition of logger and explicit typing of _status.

The addition of the logger property and the explicit typing of _status are well-implemented. These changes enhance the clarity and maintainability of the code by making dependencies explicit and improving error logging capabilities.


30-35: Updated initializer to include logger.

The initializer now correctly takes a logger parameter and uses it to initialize both the logger property and the _status instance. This change is consistent with the PR objectives to enhance logging throughout the library.


75-80: Enhanced error handling in attach method.

The addition of error logging in the attach method improves the robustness of the method by providing detailed logs in case of failures. This is a crucial enhancement for debugging and monitoring the library's behavior.


87-92: Enhanced error handling in detach method.

Similar to the attach method, the detach method now logs errors when channel operations fail. This consistent approach in error handling and logging is beneficial for maintaining the quality and reliability of the library.

Sources/AblyChat/Logging.swift (6)

9-9: Enhancement to LogLevel enum.

Making LogLevel comparable is a useful enhancement that allows for easier comparison and filtering of log levels, aligning with best practices in logging frameworks.


18-24: Addition of CodeLocation struct.

The CodeLocation struct is a well-thought-out addition that enhances the contextual information available during logging by capturing the file ID and line number.


26-36: Definition of InternalLogger protocol.

The InternalLogger protocol is appropriately defined to include a log method that requires a CodeLocation, ensuring that log messages are detailed and traceable.


38-44: Convenience method for logging.

The extension to InternalLogger adds a convenience method for logging that simplifies the process for developers by automatically capturing the file ID and line number. This is a practical enhancement that reduces boilerplate code.


46-63: Implementation of DefaultInternalLogger.

The DefaultInternalLogger class is well-implemented, providing a concrete logging mechanism that respects the log level settings. The use of a guard clause to check the log level before logging is a good practice that enhances performance by avoiding unnecessary logging operations.


66-76: Update to DefaultLogHandler.

The update to DefaultLogHandler to utilize Swift's Logger type and handle log messages based on their severity is a robust implementation that aligns with modern logging practices.

Tests/AblyChatTests/DefaultRoomTests.swift (1)

19-19: Updated DefaultRoom instantiation in tests.

The instantiation of DefaultRoom in various test cases has been correctly updated to include the logger parameter. This change is necessary to align with the modifications in the main library code and ensures that the logging functionality is integrated into the testing framework.

Also applies to: 56-56, 85-85, 122-122

Tests/AblyChatTests/InternalLoggerTests.swift Show resolved Hide resolved
Copy link
Collaborator

@maratal maratal left a comment

Choose a reason for hiding this comment

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

Left a few questions

Sources/AblyChat/Logging.swift Show resolved Hide resolved
Tests/AblyChatTests/DefaultInternalLoggerTests.swift Outdated Show resolved Hide resolved
Sources/AblyChat/Logging.swift Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ead9e26 and 888f2ea.

Files selected for processing (14)
  • Sources/AblyChat/ChatClient.swift (1 hunks)
  • Sources/AblyChat/Logging.swift (1 hunks)
  • Sources/AblyChat/Room.swift (2 hunks)
  • Sources/AblyChat/RoomStatus.swift (2 hunks)
  • Sources/AblyChat/Rooms.swift (2 hunks)
  • Tests/AblyChatTests/DefaultInternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomStatusTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomTests.swift (4 hunks)
  • Tests/AblyChatTests/DefaultRoomsTests.swift (3 hunks)
  • Tests/AblyChatTests/Helpers/SynchronizedAccess.swift (1 hunks)
  • Tests/AblyChatTests/Helpers/TestLogger.swift (1 hunks)
  • Tests/AblyChatTests/InternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockInternalLogger.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockLogHandler.swift (1 hunks)
Files skipped from review due to trivial changes (2)
  • Tests/AblyChatTests/Helpers/TestLogger.swift
  • Tests/AblyChatTests/Mocks/MockInternalLogger.swift
Files skipped from review as they are similar to previous changes (8)
  • Sources/AblyChat/Room.swift
  • Sources/AblyChat/RoomStatus.swift
  • Sources/AblyChat/Rooms.swift
  • Tests/AblyChatTests/DefaultInternalLoggerTests.swift
  • Tests/AblyChatTests/DefaultRoomStatusTests.swift
  • Tests/AblyChatTests/DefaultRoomTests.swift
  • Tests/AblyChatTests/DefaultRoomsTests.swift
  • Tests/AblyChatTests/Mocks/MockLogHandler.swift
Additional comments not posted (9)
Tests/AblyChatTests/InternalLoggerTests.swift (1)

4-16: Well-implemented test for logging functionality.

The test method test_protocolExtension_logMessage_defaultArguments_populatesFileIDAndLine is well-structured and correctly verifies the log message, level, and code location. It uses good practices such as XCTUnwrap for optional values and dynamic code location checks with #line and #fileID.

Tests/AblyChatTests/Helpers/SynchronizedAccess.swift (1)

1-30: Well-implemented property wrapper for thread-safe access.

The SynchronizedAccess property wrapper is correctly implemented to provide thread-safe access to properties using NSLock. The detailed comment at the beginning of the file is informative and helps clarify the intended use and limitations of the property wrapper.

Sources/AblyChat/ChatClient.swift (1)

17-23: Well-integrated logging functionality in DefaultChatClient.

The addition of the logger property and its initialization using clientOptions are well-implemented. The modification to include the logger in the rooms property initialization ensures that logging is integrated throughout the chat client's operations.

Sources/AblyChat/Logging.swift (6)

9-9: Enhancement: LogLevel now supports comparisons.

The addition of Comparable to LogLevel is a significant enhancement, allowing for easier comparison and filtering of log levels. This change supports more sophisticated log handling scenarios, such as adjusting log output based on the environment or user settings.


18-24: Introduction of CodeLocation struct for better logging context.

The CodeLocation struct is a well-thought-out addition that encapsulates the file identifier and line number. This will greatly enhance the debugging process by providing precise locations in the code where log messages are generated.


26-36: Review of InternalLogger protocol.

The InternalLogger protocol is designed to provide a flexible and powerful logging interface for internal SDK components. It allows detailed logging without exposing too much complexity to the SDK users. The inclusion of codeLocation in the log method is particularly useful for associating log messages with their source in the code, enhancing traceability.


38-44: Convenience method for logging with automatic code location capture.

This extension to InternalLogger adds a convenience method that automatically captures the file ID and line number. This is a practical feature that simplifies the logging process for developers, making it easier to include detailed contextual information without manual effort.


46-64: Implementation of DefaultInternalLogger.

The DefaultInternalLogger class is well-implemented, providing a concrete logging mechanism that checks the log level before logging messages. The use of dependency injection for the LogHandler and the defaulting mechanism for both the handler and log level are good practices that enhance the flexibility and testability of the logging system.


79-91: Mapping LogLevel to OSLogType.

The private extension that maps LogLevel to OSLogType is a clean and efficient way to integrate with the OS logging system. The handling of different log levels and the decision to return nil for .silent to effectively skip logging is well thought out.

Sources/AblyChat/Logging.swift Outdated Show resolved Hide resolved
The aim of this commit is to:

- provide infrastructure for internal components to be able to log
  messages
- add some log messages to the existing functionality (I am not
  particularly concerned by the quality or utility of these log messages
  at the moment)
- provide a basic default logging backend so that users can see log
  messages in the console

The choice to have a separate internal logging interface is taken from
Asset Tracking (AAT) and ably-cocoa, where I think it works well. I
haven’t currently copied across AAT’s subsystem mechanism, a large part
of the motivation for which was to provide a way to integrate the logs
emitted by the AAT-instantiated Realtime client into those emitted by
AAT itself; this is not currently a goal for Chat, which does not
instantiate a Realtime client.

I briefly toyed with the idea of writing a macro that would log the
inputs and outputs of public API methods, but figuring out how to do
that didn’t seem like a good use of time now.

Resolves #38.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 888f2ea and 4b72ffc.

Files selected for processing (14)
  • Sources/AblyChat/ChatClient.swift (1 hunks)
  • Sources/AblyChat/Logging.swift (1 hunks)
  • Sources/AblyChat/Room.swift (2 hunks)
  • Sources/AblyChat/RoomStatus.swift (2 hunks)
  • Sources/AblyChat/Rooms.swift (2 hunks)
  • Tests/AblyChatTests/DefaultInternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomStatusTests.swift (1 hunks)
  • Tests/AblyChatTests/DefaultRoomTests.swift (4 hunks)
  • Tests/AblyChatTests/DefaultRoomsTests.swift (3 hunks)
  • Tests/AblyChatTests/Helpers/SynchronizedAccess.swift (1 hunks)
  • Tests/AblyChatTests/Helpers/TestLogger.swift (1 hunks)
  • Tests/AblyChatTests/InternalLoggerTests.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockInternalLogger.swift (1 hunks)
  • Tests/AblyChatTests/Mocks/MockLogHandler.swift (1 hunks)
Files skipped from review due to trivial changes (3)
  • Sources/AblyChat/RoomStatus.swift
  • Tests/AblyChatTests/Helpers/TestLogger.swift
  • Tests/AblyChatTests/Mocks/MockInternalLogger.swift
Files skipped from review as they are similar to previous changes (9)
  • Sources/AblyChat/ChatClient.swift
  • Sources/AblyChat/Logging.swift
  • Sources/AblyChat/Rooms.swift
  • Tests/AblyChatTests/DefaultInternalLoggerTests.swift
  • Tests/AblyChatTests/DefaultRoomStatusTests.swift
  • Tests/AblyChatTests/DefaultRoomTests.swift
  • Tests/AblyChatTests/DefaultRoomsTests.swift
  • Tests/AblyChatTests/Helpers/SynchronizedAccess.swift
  • Tests/AblyChatTests/Mocks/MockLogHandler.swift
Additional comments not posted (3)
Tests/AblyChatTests/InternalLoggerTests.swift (1)

4-16: Well-implemented test for logging functionality.

The test method test_protocolExtension_logMessage_defaultArguments_populatesFileIDAndLine is correctly implemented and verifies essential aspects of the logging functionality. No further changes are necessary at this point.

Sources/AblyChat/Room.swift (2)

27-35: Good integration of logging in the DefaultRoom initializer.

The addition of the logger parameter in the initializer and its subsequent use in initializing _status is well-implemented. This change enhances the logging capabilities of the DefaultRoom actor, aligning with the PR objectives to improve visibility and error handling.


75-80: Effective error handling and logging in channel operations.

The use of do-catch blocks in the attach and detach methods to log errors and rethrow them is a robust approach to error handling. This ensures that errors are not only logged but also properly propagated, allowing higher-level handlers to manage them accordingly.

Also applies to: 87-92

Copy link
Collaborator

@maratal maratal left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Implement logging
3 participants