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

Bad warning on entering dead await #174

Open
maxtaco opened this issue Mar 30, 2016 · 11 comments
Open

Bad warning on entering dead await #174

maxtaco opened this issue Mar 30, 2016 · 11 comments
Labels

Comments

@maxtaco
Copy link
Owner

maxtaco commented Mar 30, 2016

ICED warning: Entered dead await at Mailer._render (undefined:263)
@zapu
Copy link
Collaborator

zapu commented Apr 4, 2016

Hm, what would be a good repro for dead await right now (after we fixed the race condition issue)?

@maxtaco
Copy link
Owner Author

maxtaco commented Apr 5, 2016

That is a great question... We probably have to reach inside and poke at the internals. Alternatively, we can trigger the overused deferral argument. It uses the same code path.

@maxtaco
Copy link
Owner Author

maxtaco commented Apr 5, 2016

For instance:

foo = (cb) -> cb(); cb()
await foo defer()

Outputs:

ICED warning: overused deferral at <anonymous> (undefined:3)

maxtaco pushed a commit that referenced this issue Apr 18, 2016
Traces did not get 'filename' property, which resulted in confusing
runtime warnings, like:

`ICED warning: overused deferral at <anonymous> (undefined:3)`

Fixed this by adding filename in Await.transform.

Added new tests that check for this issue.

Fixes #174
@zapu
Copy link
Collaborator

zapu commented Apr 18, 2016

So this issue is now fixed, I think. What is there left?

@maxtaco
Copy link
Owner Author

maxtaco commented Apr 18, 2016

Thanks for fixing it. I had original omitted this feature since sometimes the pen-testers like to complain about "full path disclosure" in the emitted source code. That's not the thing I lose any sleep over, but hey.

I think we should get ready to launch an alpha iced3, which you can get via npm i iced-coffee-script-3. I was just poking around in the repo and was wondering what we should do about generated lib/* files. They really do get in the way, but it would be nice to have a record of what was generated in Github for historical releases. Maybe we can have a release branch that we commit the generated .js files to?

@zapu
Copy link
Collaborator

zapu commented Apr 18, 2016

I could have fixed this earlier, had I realized that the issue was only a result of missing filename :)

Ad 1), I recall thinking about this before, but I'd say it's a common practice currently to "leak" stuff like this everywhere. It's usually full path of the developer/buildbot's machine, though. A little weird, compared to, say, proprietary c/c++ software development when they try to strip and sometimes obfuscate everything, everywhere.

We may also introduce a compiler flag that would block emitting traces and other original-source-code information. I think it's only line numbers on Defer nodes, filenames and method names on Await nodes, but I'm not actually looking at the code right now.

  1. So the workflow would be to merge things into iced3 branch, then, for release, compile everything, test, but create the actual commit on top of the release branch?

@zapu
Copy link
Collaborator

zapu commented Apr 19, 2016

So what if we change the trace to only include filename, not the full path? Or the path relative to directory where the compiler was invoked?

@maxtaco
Copy link
Owner Author

maxtaco commented Apr 19, 2016

i like the idea of the relative path, that's cleaner...

On Tue, Apr 19, 2016 at 9:02 AM, Michał Zochniak [email protected]
wrote:

So what if we change the trace to only include filename, not the full
path? Or the path relative to directory where the compiler was invoked?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#174 (comment)

@zapu
Copy link
Collaborator

zapu commented Apr 25, 2016

Or maybe it should be responsibility of the tools (like icsify... I don't know what else is used) to do this?

What also could be useful is detecting function names in functions like:

foo = ->
    foo = (cb) -> cb(); cb()
    await foo defer()

foo()

Right now the error is ICED warning: overused deferral at <anonymous> which is not wrong, but it could try to do better work at. Function names work correctly if the error happens in a class method.

@zapu
Copy link
Collaborator

zapu commented May 2, 2016

@maxtaco what do you think? So should we try to do an alpha release? Maybe @doublerebel would be able to try to test his code base on the new version. Then build tools could be adjusted to work better with iced3. I've only ever used gulp and browserify, so I'm very much clueless in what's popular right now.

@maxtaco
Copy link
Owner Author

maxtaco commented May 2, 2016

OK, let's punt on all of the downstream usage issues. I think the one last remaining piece is a package/release script. There are enough things to remember that we're bound to forget if we don't script it. Ticket here: #180

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

No branches or pull requests

2 participants