Logging breaks with binary output or between multi-byte UTF-8 characters
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 60
- Forks
- 9
- Avg merge
- 3h 59m
- Merged PRs (30d)
- 8
Description
...and also introduces significant performance overhead.
Currently the logger a) attempts to parse all yielded segments of data as UTF-8, and b) calls logger.debug with all output. This causes a few problems.
- If the output of the command, on stdout or stderr, is non-decodable binary data (e.g. an image), the
String(data:encoding:)will fail. This doesn't actually cause any issues, it just returns nil and fails to log, but it does mean that the logging silently doesn't log anything. - That process incurs an overhead. If calling with a significant amount of data (megabytes or more), this introduces a significant performance penalty.
- Even if the output on stdout or stderr of the command is UTF-8, this also does not work as expected because the process may not be yielding data aligned to UTF-8 codepoint boundaries. For example, an emoji may be two bytes, if these bytes were yielded separately, two incorrect strings would be produced instead of one correct emoji.
I see the reason for logging, but I think this currently makes the library less usable in different use-cases. I think a good option would be to:
- Make this logging optional, beyond debug log level (as this may still be a useful level for those reading non-UTF-8 data), with a flag on the class that disables any attempt to parse the bytes.
- When enabled, attempt to parse as a string, but fall back to printing the bytes, perhaps in hexadecimal format.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
No file or test is named. Start by locating the process-output logging path that parses yielded stdout and stderr segments and calls logger.debug. Done means logging can be disabled without parsing bytes, while enabled logging handles non-UTF-8 data and UTF-8 characters split across yields without the current performance cost.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100