Skip to content

Proposal: Fix NTDebugHandler and Deprecate setup_logging in logutil.py. #920

@junkmd

Description

@junkmd

Summary

The logutil module contains components that are either broken, or overly broad in this project responsibilities, and have not been properly maintained or tested for a long time.
I propose to fix the NTDebugHandler class and deprecate the setup_logging function.

Problems

1. NTDebugHandler is broken due to Python 2/3 str vs bytes incompatibility

class NTDebugHandler(logging.Handler):
def emit(
self,
record,
writeA=_OutputDebugStringA,
writeW=_OutputDebugStringW,
):
text = self.format(record)
if isinstance(text, str):
writeA(text + "\n")
else:
writeW(text + "\n")

  • In Python 2, str represented byte sequences, and Unicode strings were handled by a separate unicode type.
    In Python 3, str represents Unicode text, and bytes is used explicitly for byte sequences.
  • In Python 3, logging.Formatter.format, which is invoked by logging.Logger.format, always returns a Unicode string (str).
  • The NTDebugHandler.emit method has a type check (isinstance(text, str)) and passing this str directly to _OutputDebugStringA (which expects LPCSTR(bytes)) inevitably leads to errors.
  • So _OutputDebugStringW (for wide character strings) is sufficient. The existing type branching is now a source of malfunction.

2. setup_logging has excessive responsibility

  • The setup_logging is a just logging utility, unrelated to COM or Windows.
  • Such logging configuration responsibilities should ideally rest with the application developer.

3. Lack of usage and redundancy

  • These components are likely unused or used in very limited circumstances for a long time.
  • NTDebugHandler is used in server/inprocserver.py, but as discussed in Are there any use cases for DllCanUnloadNow and DllGetClassObject? #671, the use case for inprocserver has not been confirmed for years.
    This suggests either that its breakage has gone unnoticed, or developers are using forks/custom patches.
  • The setup_logging function is not called anywhere within server/inprocserver.py, indicating its non-essential nature for the project and a weak justification for its existence.

Proposal

For the reasons outlined above, we propose to fix NTDebugHandler and deprecate the setup_logging function from this library.
This will simplify the codebase, resolve potential Python version incompatibility issues, and clarify the scope of this project responsibilities.

I plan to fix NTDebugHandler by the next release and deprecate setup_logging, with a view to removing it in a future version (likely 1.5.0).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtestsenhance or fix tests

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions