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

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions