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

Show stdout/stderr from Wasm in terminal #472

Merged
merged 3 commits into from
May 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 81 additions & 39 deletions Sources/CartonKit/Server/Server.swift
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,14 @@ public actor Server {
let environment =
head.headers["User-Agent"].compactMap(DestinationEnvironment.init).first
?? .other
let handler = await ServerWebSocketHandler(
let handler = ServerWebSocketHandler(
configuration: ServerWebSocketHandler.Configuration(
onText: self.createWebSocketTextHandler(in: environment, terminal: self.configuration.terminal)
onText: { [weak self] (text) in
self?.webSocketTextHandler(text: text, environment: environment)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

処理は普通にメソッドで書いて、weak self を明示的に書いた方が、
参照がどうなっているかと、メソッドで何をするか、
という独立な要素がコードとして分離できてシンプルです。

},
onBinary: { [weak self] (data) in
self?.webSocketBinaryHandler(data: data)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

新たにバイナリメッセージのハンドリングを追加します。

}
)
)
await self.add(connection: Connection(channel: channel))
Expand Down Expand Up @@ -329,50 +334,87 @@ public actor Server {
}

extension Server {
/// Returns a handler that responds to WebSocket messages coming from the browser.
func createWebSocketTextHandler(
in environment: DestinationEnvironment,
terminal: InteractiveWriter
) -> @Sendable (String) -> Void {
{ [weak self] text in
guard let self = self else { return }
guard
let data = text.data(using: .utf8),
let event = try? self.decoder.decode(Event.self, from: data)
else {
return
}
/// Respond to WebSocket messages coming from the browser.
nonisolated func webSocketTextHandler(
Copy link
Contributor Author

@omochi omochi May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここなんですが、関数を返す関数から、普通のメソッドにしました。

まず、いきなり大きなラムダ式を開くコードはややこしいです。
全体に余計なインデントがずっと入るし、
メソッドとラムダの間で他のインタラクトに気をつけて読まないといけません。

通常のメソッドにした方が、 actor のメソッドとしての解析や制御がちゃんと機能します。

selfの弱参照についても、
根本のクロージャで書いた方がどういうポリシーで管理しているかわかりやすいでしょう。

このメソッド自体についてですが、
同期のクロージャから呼びたいので nonisolated にしています。

text: String,
environment: DestinationEnvironment
) {
guard
let data = text.data(using: .utf8),
let event = try? self.decoder.decode(Event.self, from: data)
else {
return
}

switch event {
case let .stackTrace(rawStackTrace):
if let stackTrace = rawStackTrace.parsedStackTrace(in: environment) {
terminal.write("\nAn error occurred, here's a stack trace for it:\n", inColor: .red)
stackTrace.forEach { item in
terminal.write(" \(item.symbol)", inColor: .cyan)
terminal.write(" at \(item.location ?? "<unknown>")\n", inColor: .gray)
}
} else {
terminal.write("\nAn error occurred, here's the raw stack trace for it:\n", inColor: .red)
terminal.write(
" Please create an issue or PR to the Carton repository\n"
+ " with your browser name and this raw stack trace so\n"
+ " we can add support for it: https://github.com/swiftwasm/carton\n", inColor: .gray
)
terminal.write(rawStackTrace + "\n")
let terminal = self.configuration.terminal
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self から取ってきます。


switch event {
case let .stackTrace(rawStackTrace):
if let stackTrace = rawStackTrace.parsedStackTrace(in: environment) {
terminal.write("\nAn error occurred, here's a stack trace for it:\n", inColor: .red)
stackTrace.forEach { item in
terminal.write(" \(item.symbol)", inColor: .cyan)
terminal.write(" at \(item.location ?? "<unknown>")\n", inColor: .gray)
}
} else {
terminal.write("\nAn error occurred, here's the raw stack trace for it:\n", inColor: .red)
terminal.write(
" Please create an issue or PR to the Carton repository\n"
+ " with your browser name and this raw stack trace so\n"
+ " we can add support for it: https://github.com/swiftwasm/carton\n", inColor: .gray
)
terminal.write(rawStackTrace + "\n")
}

case let .testRunOutput(output):
TestsParser().parse(output, terminal)

case .testPassed:
Task { await self.stopTest(hadError: false) }

case let .errorReport(output):
terminal.write("\nAn error occurred:\n", inColor: .red)
terminal.write(output + "\n")

case let .testRunOutput(output):
TestsParser().parse(output, terminal)
Task { await self.stopTest(hadError: true) }
}
}

private static func decodeLines(data: Data) -> [String] {
let text = String(decoding: data, as: UTF8.self)
return text.components(separatedBy: .newlines)
}

nonisolated func webSocketBinaryHandler(data: Data) {
let terminal = self.configuration.terminal

if data.count < 2 {
return
}

var kind: UInt16 = 0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2バイトのヘッダをつけるフォーマットにしました。

_ = withUnsafeMutableBytes(of: &kind) { (buffer) in
data.copyBytes(to: buffer, from: 0..<2)
}

case .testPassed:
Task { await self.stopTest(hadError: false) }
switch kind {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reinterpret with UInt16(littleEndian:) to ensure the bytes are interpreted as little endian

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど。そういうのがあるんですね。やりました。

case 1001:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1001はstdoutのチャンクを転送するコマンドです

// stdout
let chunk = data.subdata(in: 2..<data.count)
if chunk.isEmpty { return }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

空の書き込みが来ることがあるので捨てます。


case let .errorReport(output):
terminal.write("\nAn error occurred:\n", inColor: .red)
terminal.write(output + "\n")
for line in Self.decodeLines(data: chunk) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここまではバイナリで来てますが、
最後のこの場所でコンソールに綺麗に出すために文字列処理します。

terminal.write("stdout: " + line + "\n")
}
case 1002:
// stderr
let chunk = data.subdata(in: 2..<data.count)
if chunk.isEmpty { return }

Task { await self.stopTest(hadError: true) }
for line in Self.decodeLines(data: chunk) {
terminal.write("stderr: " + line + "\n", inColor: .red)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラーは赤くしたんですが色がつきませんでした。
SwiftPMの問題。
swiftlang/swift-package-manager#7589

}
default: break
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions Sources/CartonKit/Server/ServerWebSocketHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import Foundation
import NIO
import NIOWebSocket

Expand All @@ -20,7 +21,8 @@ final class ServerWebSocketHandler: ChannelInboundHandler {
typealias OutboundOut = WebSocketFrame

struct Configuration {
let onText: @Sendable (String) -> Void
var onText: @Sendable (String) -> Void
var onBinary: @Sendable (Data) -> Void
}

private var awaitingClose: Bool = false
Expand All @@ -43,7 +45,11 @@ final class ServerWebSocketHandler: ChannelInboundHandler {
var data = frame.unmaskedData
let text = data.readString(length: data.readableBytes) ?? ""
self.configuration.onText(text)
case .binary, .continuation, .pong:
case .binary:
let nioData = frame.unmaskedData
let data = Data(nioData.readableBytesView)
self.configuration.onBinary(data)
case .continuation, .pong:
// We ignore these frames.
break
default:
Expand Down
17 changes: 16 additions & 1 deletion Tests/Fixtures/DevServerTestApp/Sources/app/main.swift
Original file line number Diff line number Diff line change
@@ -1 +1,16 @@
print("hello dev server")
#if os(WASI)
import WASILibc
typealias FILEPointer = OpaquePointer
#else
import Darwin
typealias FILEPointer = UnsafeMutablePointer<FILE>
#endif

func fputs(_ string: String, file: FILEPointer) {
_ = string.withCString { (cstr) in
fputs(cstr, file)
}
}

fputs("hello stdout\n", file: stdout)
fputs("hello stderr\n", file: stderr)
20 changes: 20 additions & 0 deletions entrypoint/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,29 @@ const startWasiTask = async () => {

const wasmRunner = WasmRunner(
{
onStdout(chunk) {
const kindBuffer = new ArrayBuffer(2);
new DataView(kindBuffer).setUint16(0, 1001, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

頭に2バイトのヘッダをつけてからwebsocketに流します。
リトルエンディアンにするので true を指定しています。

JSのバイナリ処理に慣れてなくて書き方がこれでいいか自信がないです。
なんか冗長です。


const buffer = new Uint8Array(2 + chunk.length);
buffer.set(new Uint8Array(kindBuffer), 0);
buffer.set(chunk, 2);

socket.send(buffer);
},
onStdoutLine(line) {
console.log(line);
},
onStderr(chunk) {
const kindBuffer = new ArrayBuffer(2);
new DataView(kindBuffer).setUint16(0, 1002, true);

const buffer = new Uint8Array(2 + chunk.length);
buffer.set(new Uint8Array(kindBuffer), 0);
buffer.set(chunk, 2);

socket.send(buffer);
},
onStderrLine(line) {
console.error(line);
}
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.