src: handle report options on fatalerror #32497
Conversation
nodejs-github-bot
added
c++
Which is kindof the point... I'll look into suppressing that. And it seems the markdown linter has dozens of complaints about the collaborator and cpp style guides. |
|
@sam-github - do you have a test case / story from users that reproduce the issue you are solving here? |
Getting rid of the
Where are you seeing those? |
|
@gireeshpunathil Its the same case as previous, the previous fix was to make it posible to check the value of --report-on-fatalerror. Now that is checked, and the value of report-on-fatalerror is respected. But actually triggering the report requires access to 3 other cli report values (filename, dirname, compact). Those aren't reachable under the same conditions that report-on-fatalerror did not used to be reachable. I'll extend the tests to show this. |
sam-github
force-pushed
the
fix-report-options
branch
from
cca1bae
to
e5cd075
Compare
|
@gireeshpunathil I expanded the test suite, run with a version of node predating this and you can see what used to fail. |
sam-github
force-pushed
the
fix-report-options
branch
from
e5cd075
to
7400dd2
Compare
|
@nodejs/diagnostics PTAL |
|
Recurring failure is unrelated: #32510 |
There was a problem hiding this comment.
As @cjihrig pointed out in #32535 (review), this is missing guards to protect against race conditions.
Follow on to nodejs#32207, 3 other options are also not respected under situations that the isolate is not available.
sam-github
force-pushed
the
fix-report-options
branch
from
7400dd2
to
921d4e0
Compare
|
@addaleax I have implemented the locking, PTAL |
Follow on to #32207, 3 other options are also not respected under situations that the isolate is not available. PR-URL: #32497 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Follow on to #32207, 3 other options are also not respected under situations that the isolate is not available. PR-URL: #32497 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Follow on to nodejs#32207, 3 other options are also not respected under situations that the isolate is not available. PR-URL: nodejs#32497 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Follow on to #32207, 3 other options are also not respected under situations that the isolate is not available. PR-URL: #32497 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Follow on to #32207, 3 other options
are also not respected under situations that the isolate is not
available.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes