-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Send stdout/stderr from WasmRunner to dev server without decoding as UTF-8 #471
Conversation
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.
self comment
const wasmRunner = WasmRunner(false, runtimeConstructor); | ||
const wasmRunner = WasmRunner({ | ||
onStdoutLine(line) { | ||
console.log(line); |
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.
コンソールに流す処理はWasmRunnerのユーザの裁量にします。
entrypoint/bundle.ts
Outdated
try { | ||
await startWasiTask(); | ||
} catch (e) { | ||
handleError(e); |
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.
handleErrorの経路が同期と非同期で2回書いてあるのは冗長だし読みづらいので、
try await catch で1つにします。
@@ -40,13 +47,14 @@ const startWasiTask = async () => { | |||
|
|||
function handleError(e: any) { | |||
console.error(e); | |||
if (e instanceof WebAssembly.RuntimeError) { | |||
console.log(e.stack); |
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.
エラーオブジェクトがconsoleに渡った場合はそこからスタックトレースも出てくるのでこの行は不要です
何もしなければブラウザの例外ダンプが出るのでそれが一番いい
@@ -15,35 +15,72 @@ | |||
import { WASI, File, OpenFile, ConsoleStdout, PreopenDirectory } from "@bjorn3/browser_wasi_shim"; | |||
import type { SwiftRuntime, SwiftRuntimeConstructor } from "./JavaScriptKit_JavaScriptKit.resources/Runtime"; | |||
|
|||
export class LineDecoder { |
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.
bjorn3/browser_wasi_shim
を参考に書き直しました。
onStdout?: (text: string) => void; | ||
onStderr?: (text: string) => void; | ||
onStdout?: (chunk: Uint8Array) => void; | ||
onStdoutLine?: (line: string) => void; |
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.
onStdoutはバイナリAPIに変えます。
onStdoutLineでは行単位で受け取れます。
const defaultRunnerOptions = (options: Options | false): Options => { | ||
if (!options) return defaultRunnerOptions({}); | ||
if (!options.onStdout) { | ||
options.onStdout = () => { }; |
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.
このあたり無意味だと思ったので消しました。
); | ||
Error.stackTraceLimit = prevLimit; | ||
onStdoutLine(line) { | ||
console.log(line); |
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.
従来のコンソールに流す挙動はユーザ側で再現します。
kind: "stackTrace", | ||
stackTrace: stack, | ||
}) | ||
); |
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.
スタックを送信する処理は handleError の時だけにします。
でもこれだと非同期で後から死んだ場合が漏れるなあ
if (e instanceof WebAssembly.RuntimeError) { | ||
console.log(e.stack); | ||
|
||
if (e instanceof Error) { |
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.
Wasmの実行が止まったら handleError
にくるので、
そこで例外をサーバに送信します。
dev.jsがデグレしているので直します。 |
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.
update
@@ -0,0 +1,4 @@ | |||
This application serves as a working environment for experimenting with the behavior of "carton". |
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.
SandboxAppを追加します。
自動テストからは利用されないが、手で起動して実験するためのアプリです。
本当は全部自動テストするのが望ましいですが、
技術的に難しくて自動化をすぐに用意できないシナリオもしばしばあるので、
こういうのがあったほうが良いと思いました。
特に実際には、
ボタンを押したら fatalError
によって Swiftプロセスが終了する時の、
dev.jsのスタックトレースの転送を調べていました。
}); | ||
await startWasiTask(); | ||
} catch (e) { | ||
handleError(e); |
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.
Swiftのmain関数でクラッシュした場合 (JS的には例外送出)と、
Windowのエラーハンドラ(ボタンクリック時など)と、
エラーになったPromiseが次に渡されなかったとき、
にスタックトレースを送信します。
デフォルトのブラウザの挙動としてコンソールにダンプが出ますが、
handleError
のなかの console.error
と内容がかぶるので preventDefault
します。
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.
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.
なるほど。
onerrorとonrejectなんとかは preventDefaultしないでブラウザが出す方に任せたら良いのか。
そうします。
mainの方はcatchして再throwしてもトレースは繋がるんだろうか。
試してみます。
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.
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.
@@ -65,14 +60,32 @@ const startWasiTask = async () => { | |||
}; | |||
|
|||
function handleError(e: any) { | |||
console.error(e); |
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.
コンソールに流す処理はやめます。
そのままブラウザに例外を渡せば出力してくれるからです。
async function main(): Promise<void> { | ||
try { | ||
window.addEventListener("error", (event) => { | ||
handleError(event.error); |
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.
スタックトレースの送信だけして、イベントはそのままにします。
そのまま自然にブラウザの例外ダンプを引き起こしたいため。
await startWasiTask(); | ||
} catch (e) { | ||
handleError(e); | ||
throw e; |
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.
再throwすることでブラウザによってダンプされます。
try { | ||
await startWasiTask(); | ||
} catch (e) { | ||
handleError(e); |
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.
テストのエラーハンドリングは複雑で壊しそうだったのでここでは
try-await-catchにするだけで例外データフローは変えません。
@kateinoigakukun ブラウザの例外ダンプを使う提案を採用しました。 |
背景
entrypointライブラリの
WasmRunner
は、stdoutとstderrをWasmから受け取って、呼び出し側のJSから扱いやすいようにクロージャを受け取るAPIを提供しています。
また、dev.jsではWasmプロセスが例外を吐いて停止した時にスタックトレースをdevサーバに送信する機能を持ちます。
課題
このAPIは、WasmのバイナリをUTF-8ととしてデコードして、
改行で区切って単位で早出します。
これだと、UTF-8の文字列以外の出力が扱えません。
また、エラーを出力する処理が3重に存在していて、エラーが起きた時の出力が冗長です。
また、stderrに少しでも出力があると例外を送信してしまいます。
提案
生のバイナリチャンクを受け取れるAPIと、
従来のように文字列を行単位で受け取るAPIの両方を提供するように変更します。
余計なコンソール出力をやめます。
例外の送信はWasmの終了時だけにします。
また、この作業をしていて、
cartonの機能を確認するための十分なサンプル実装がないと思ったので、
SandboxApp
を追加しました。動作確認
devモードについてはスタックトレースの送出タイミングを変えたので動作を確認しています。
これは SwiftでfatalError を呼び出すだけのボタンをクリックした時のブラウザとターミナルのコンソールです。
ブラウザの一つ目は stderr の行が console.error で転送された出力です。
ブラウザの二つ目は dev.js でつかまえて console.error で出力されたエラーオブジェクトです。
これをdev serverに送信します。
コンソールです。