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
improve string interpolation of errors #6482
Conversation
…ion on errors motivation: better error messages changes: * audit usage of "\(error)" and replace them with "\(error.interpolationDescription)" * add ADPI to pass an optional underlying error to observability's debug, info and warn calls such that it automatically interpolates the error correctly * update call sites rdar://108418554
@swift-ci smoke test |
@swift-ci smoke test windows |
Could you call out the core changes? Looks like a look of formatting stuff which is difficult to wade through. |
@@ -173,45 +174,69 @@ extension DiagnosticsEmitterProtocol { | |||
self.emit(.init(severity: severity, message: message, metadata: metadata)) | |||
} | |||
|
|||
public func emit(error message: String, metadata: ObservabilityMetadata? = .none) { | |||
public func emit(error message: String, metadata: ObservabilityMetadata? = .none, underlyingError: Error? = .none) { | |||
let message = makeMessage(from: message, underlyingError: underlyingError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main change here
} | ||
|
||
public func emit(info message: String, metadata: ObservabilityMetadata? = .none) { | ||
public func emit(info message: String, metadata: ObservabilityMetadata? = .none, underlyingError: Error? = .none) { | ||
let message = makeMessage(from: message, underlyingError: underlyingError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main change here
} | ||
|
||
public func emit(debug message: String, metadata: ObservabilityMetadata? = .none) { | ||
public func emit(debug message: String, metadata: ObservabilityMetadata? = .none, underlyingError: Error? = .none) { | ||
let message = makeMessage(from: message, underlyingError: underlyingError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main change here
@@ -236,6 +261,14 @@ extension DiagnosticsEmitterProtocol { | |||
return false | |||
} | |||
} | |||
|
|||
private func makeMessage(from message: String, underlyingError: Error?) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main change here
} else { | ||
message = "\(error)" | ||
message = error.interpolationDescription |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main change here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only minor doc comment nit, otherwise LGTM
Long term I wonder if we could introduce a new |
Co-authored-by: Max Desiatov <m_desiatov@apple.com>
@swift-ci smoke test |
@swift-ci smoke test windows |
@swift-ci smoke test windows |
motivation: better error messages changes: * audit usage of "\(error)" and replace them with "\(error.interpolationDescription)" * add ADPI to pass an optional underlying error to observability's debug, info and warn calls such that it automatically interpolates the error correctly * update call sites rdar://108418554
motivation: better error messages changes: * audit usage of "\(error)" and replace them with "\(error.interpolationDescription)" * add ADPI to pass an optional underlying error to observability's debug, info and warn calls such that it automatically interpolates the error correctly * update call sites rdar://108418554
motivation: better error messages
changes:
rdar://108418554