Skip to content
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

Fix JSONMessageStreamingParser error message formatting #398

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Fix JSONMessageStreamingParser error message formatting #398

merged 1 commit into from
Mar 10, 2023

Conversation

tristanlabelle
Copy link
Contributor

@tristanlabelle tristanlabelle commented Mar 6, 2023

When json parsing fails in JSONMessageStreamingParser, the error thrown was meant to include the message that failed to parse, but it referred to a buffer that had already been cleared by the buffer.removeAll() line.

This is especially useful to debug swiftc crashes when invoked through swift build, where the call stack should be in the JSON message that is not printed between the two colons : : :

error: failed parsing the Swift compiler output: unexpected JSON message: : dataCorrupted(Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "kind", intValue: nil)], debugDescription: "invalid kind", underlyingError: nil))

@tristanlabelle tristanlabelle marked this pull request as ready for review March 6, 2023 20:48
@tristanlabelle tristanlabelle changed the title Fix json parsing error message formatting Fix JSONMessageStreamingParser error message formatting Mar 6, 2023
@neonichu
Copy link
Contributor

neonichu commented Mar 6, 2023

@swift-ci please smoke test

1 similar comment
@compnerd
Copy link
Member

compnerd commented Mar 8, 2023

@swift-ci please smoke test

@tristanlabelle
Copy link
Contributor Author

Ping. Any blockers for merging this? I'm seeing that Linux/macOS tests are not running.

@compnerd
Copy link
Member

compnerd commented Mar 9, 2023

@swift-ci please test Linux platform

@neonichu
Copy link
Contributor

neonichu commented Mar 9, 2023

@swift-ci please smoke test macOS

@neonichu
Copy link
Contributor

neonichu commented Mar 9, 2023

Oh I see, since this is TSC, the "smoke test" trigger doesn't do anything

@neonichu
Copy link
Contributor

neonichu commented Mar 9, 2023

@swift-ci please test macOS

@compnerd
Copy link
Member

@swift-ci please test macOS platform

@tristanlabelle
Copy link
Contributor Author

The macOS CI appears to be hitting this unrelated failure:

/Users/ec2-user/jenkins/workspace/pr-swift-tools-support-core-macos/branch-main/swift-package-manager: error: package at '/Users/ec2-user/jenkins/workspace/pr-swift-tools-support-core-macos/branch-main/swift-package-manager' is using Swift tools version 5.7.0 but the installed version is 5.5.0

@compnerd
Copy link
Member

@swift-ci please test macOS platform

@tomerd
Copy link
Contributor

tomerd commented Mar 10, 2023

@swift-ci smoke test

@tomerd
Copy link
Contributor

tomerd commented Mar 10, 2023

@shahmishal this is another CI job that needs upgrading, here this may be a more urgent need since we dont have the secondary job that uses the just-built-compiler

@compnerd
Copy link
Member

@swift-ci please test macOS platform

@DougGregor
Copy link
Member

It should be safe to merge this

@compnerd
Copy link
Member

@DougGregor agreed, but that requires mighty powers (perhaps @Azoy can help with that?)

@DougGregor DougGregor merged commit cab96f6 into swiftlang:main Mar 10, 2023
@DougGregor
Copy link
Member

💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants