-
Notifications
You must be signed in to change notification settings - Fork 568
Do not panic in ‘os’ module if argv.Length() is 0 #1117
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
Do not panic in ‘os’ module if argv.Length() is 0 #1117
Conversation
Hi, @samhocevar , thanks for the contribution. To help understand the implications, can you explain when you ran into |
Sure; I built the following Go program: func main() {
fmt.Printf("%v", len(os.Args))
} I am using I tweaked the GopherJS-generated <script src="node_modules/browserfs/dist/browserfs.js"></script>
<script src="src.js"></script>
<script type="text/javascript">
BrowserFS.install(window);
BrowserFS.configure({ fs: "XmlHttpRequest", options: { index: "index.json" }, }, function(e) { if (e) { throw e; }
window.fs = BrowserFS.BFSRequire('fs');
window.fs.constants = { O_WRONLY: 1, O_RDWR: 2, O_CREAT: 64, O_TRUNC: 512, O_APPEND: 1024, O_EXCL: 128 };
my_code();
});
</script> The stacktrace looks like this:
|
@samhocevar this change looks good to me and I don't mind merging it, but I would like to better understand why it is needed in the first place. In a browser, I wouldn't expect it make it past the Do you know where argv gets set? Or could you attach an archive or a git repo which reproduces the crash so that I could poke at it? |
Hi! I have investigated this issue more thoroughly, and I believe this was all caused by me calling the main GopherJS-generated function without arguments: |
No worries, glad you figured it out! |
@nevkontakte Hi, when would this be released? See mvdan/sh#864 |
@JounQin our plan was to release it with GopherJS 1.18. It was merged after #1111, which is a big enough of a change that we didn't feel appropriate to release as a patch-level version. So far I haven't had much time to really dig into 1.18, but I have a vacation coming up, so that should change :) If you are comfortable doing so, you should be able to install the unreleased GopherJS version from master: |
No description provided.