Logging function autoclosures aren't "rethrows"

[sjmadsen]

Expected behavior

If I have a throwing function fn() or a throwing property accessor, it would be nice if these two log statements work:

try logger.info("calling fn = \(fn())")
try logger.info("value of property = \(object.property)")

Actual behavior

Right now it does not compile, giving the error Property access can throw, but it is executed in a non-throwing autoclosure.

I believe marking the autoclosures with rethrows will resolve this problem.

Steps to reproduce

1. Per above, try a logging function with a string interpolation containing a throwing function call or property access.

If possible, minimal yet complete reproducer code (or URL to code)

I can provide something if it's really necessary, but I think there is enough information above.

SwiftLog version/commit hash

1.5.2

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.75.2 Apple Swift version 5.8 (swiftlang-5.8.0.124.2 clang-1403.0.22.11.100) Target: arm64-apple-macosx13.0 Darwin smadsen-MacBook-Pro 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000 arm64

[tomerd]

@ktoso @weissi iirc this change would be API breaking?

[weissi]

Technically it's API breaking because if you have

func foo(_ x: @autoclosure () throws -> String) rethrows {
    print(try x())
}

func oldFoo(_ x: @autoclosure () -> String) {
    print(x())
}

then you can do

let a: (@autoclosure () -> String) -> Void = oldFoo

but not

let a: (@autoclosure () -> String) -> Void = foo

(because it would drop the rethrows...)

[ktoso]

Hmm, we also can't introduce this via more overloads, that gets ambiguous very quickly.

So it's either risking it, or calling a major... This probably isn't worth a major release though.

[sjmadsen]

Major releases aren't a limited resource.

[ktoso]

They are problematic though for a package such as swift-log that is depended on by almost all high level frameworks and libraries in the server ecosystem, so we need to consider bumping major versions carefully. This isn't quite the same as it is in "leaf" packages.

[tomerd]

we could add a new set of [re]throwing APIs but that will double the API surface area. or we can just add a single log(throwing: ...) one with no sugar, e.g

 public func log(
  level: Logger.Level,
  throwing: @autoclosure () throws -> Logger.Message,
  metadata: @autoclosure () throws -> Logger.Metadata? = nil,
  source: @autoclosure () throws -> String? = nil,
  file: String = #file, function: String = #function, line: UInt = #line
) rethrows {
  ...
}

[ktoso]

Hm yes... I wonder if that's helpful enough, wdyt @sjmadsen ?

Personally I'd be interested to see if we'd actually break any oss package.. though ofc closed source users are a risk 🤔 if we did introduce rethrows... we could try to do a "community build" and see what breaks 🤔

[sjmadsen]

log(throwing: ...) is obviously not ideal, but it is certainly better than nothing.