-
Notifications
You must be signed in to change notification settings - Fork 75
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
Validate build_web_compilers, Exception handling #16
Conversation
onError: errorPort.sendPort, | ||
automaticPackageResolution: true); | ||
} on IsolateSpawnException { | ||
exitPort.close(); |
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.
@natebosch – should do this on build_runner
, too – ensures the process actually exits!
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.
hrm...or maybe this should be try/finally....
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 finally would maybe be better.
What is a case where IsoalteSpawnException
would get raised?
If package:isolate
abstracts any of this for us we should make it a priority to migrate - I don't like having this much duplication
webdev/CHANGELOG.md
Outdated
## 0.1.1 | ||
|
||
- Checks for a dependency on `build_web_compilers`. | ||
|
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.
I still don't understand why some people like <li><p>stuff</p></li>
instead of <li>stuff</li>
...
onError: errorPort.sendPort, | ||
automaticPackageResolution: true); | ||
} on IsolateSpawnException { | ||
exitPort.close(); |
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 finally would maybe be better.
What is a case where IsoalteSpawnException
would get raised?
If package:isolate
abstracts any of this for us we should make it a priority to migrate - I don't like having this much duplication
@natebosch – added two more commits – while I was at it. PTAL |
await process.shouldExit(78); | ||
}); | ||
|
||
test('should fail gracefully if there is an isolate error', () async { |
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.
where are we causing the isolate 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.
I synthesized one to make sure we "do the right thing".
For the script runner, I just use a bad .packages
file.
For the main runner, I gave it a bad file to execute...
107e6d4
to
fd42c7b
Compare
No description provided.