src: Handle NULL env scenario #32230
Conversation
nodejs-github-bot
added
the
c++
|
I'm not familiar with modules code base, so maybe this suggestion doesn't apply here, but can/should we add a DCHECK before the return, so |
|
@mmarchini If we were okay with having this crash, I think the CHECK could just stay. Maybe to provide a bit of context, situations in which (That doesn’t mean that we have a “good” story for the embedder + ESM situation. That would be a larger conversation, I imagine.) |
|
@addaleax said:
Is this blocking? It's not clear |
|
No, I think this is good to land. Was just trying to get a green CI first |
mmarchini
added
the
author ready
PR-URL: #32230 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
|
Landed in 960df3f |
PR-URL: #32230 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
PR-URL: #32230 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
PR-URL: #32230 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
related commit e9fa5ae
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes