-
Notifications
You must be signed in to change notification settings - Fork 314
When sourcekitd crashes, log the file contents with which it crashed and the request #933
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
/// Splits `message` on newline characters such that each chunk is at most `maxChunkSize` bytes long. | ||
/// | ||
/// The intended use case for this is to split compiler arguments into multiple chunks so that each chunk doesn't exceed | ||
/// the maximum message length of `os_log` and thus won't get truncated. | ||
/// | ||
/// - Note: This will only split along newline boundary. If a single line is longer than `maxChunkSize`, it won't be | ||
/// split. This is fine for compiler argument splitting since a single argument is rarely longer than 800 characters. | ||
public func splitLongMultilineMessage(message: String, maxChunkSize: Int = 800) -> [String] { | ||
var chunks: [String] = [] | ||
for line in message.split(separator: "\n", omittingEmptySubsequences: false) { | ||
if let lastChunk = chunks.last, lastChunk.utf8.count + line.utf8.count < maxChunkSize { | ||
chunks[chunks.count - 1] += "\n" + line | ||
} else { | ||
chunks.append(String(line)) | ||
} | ||
} | ||
return chunks | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,11 @@ extension SourceKitD { | |
|
||
// MARK: - Convenience API for requests. | ||
|
||
public func send(_ req: SKDRequestDictionary) async throws -> SKDResponseDictionary { | ||
/// - Parameters: | ||
/// - req: The request to send to sourcekitd. | ||
/// - fileContents: The contents of the file that the request operates on. If sourcekitd crashes, the file contents | ||
/// will be logged. | ||
public func send(_ req: SKDRequestDictionary, fileContents: String?) async throws -> SKDResponseDictionary { | ||
logRequest(req) | ||
|
||
let signposter = logger.makeSignposter() | ||
|
@@ -86,6 +90,24 @@ extension SourceKitD { | |
|
||
guard let dict = sourcekitdResponse.value else { | ||
signposter.endInterval("sourcekitd-request", signposterState, "Error") | ||
if sourcekitdResponse.error == .connectionInterrupted { | ||
let log = """ | ||
Request: | ||
\(req.description) | ||
|
||
File contents: | ||
\(fileContents ?? "<nil>") | ||
""" | ||
let chunks = splitLongMultilineMessage(message: log) | ||
for (index, chunk) in chunks.enumerated() { | ||
logger.fault( | ||
""" | ||
sourcekitd crashed (\(index + 1)/\(chunks.count)) | ||
\(chunk) | ||
""" | ||
) | ||
} | ||
} | ||
throw sourcekitdResponse.error! | ||
} | ||
|
||
|
@@ -95,9 +117,7 @@ extension SourceKitD { | |
} | ||
|
||
private func logRequest(_ request: SKDRequestDictionary) { | ||
// FIXME: Ideally we could log the request key here at the info level but the dictionary is | ||
// readonly. | ||
logger.log( | ||
logger.info( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's default vs info? The docs weren't particularly illuminating. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, so persisted vs not 👍 |
||
""" | ||
Sending sourcekitd request: | ||
\(request.forLogging) | ||
|
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.
If OSLog treats this as private (which I believe is the default?), will we just log multiple
sourcekitd crashed
lines with redacted chunks?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.
Yes, we will. It’s a big unfortunate but I don’t have any better idea.