fix: Support absence of TextEncoder/TextDecoder/console #1820
Conversation
| @@ -174,6 +174,10 @@ freeze(ErrorInfo); | |||
|
|
|||
| /** @type {MakeCausalConsole} */ | |||
There was a problem hiding this comment.
Please adjust the type MakeCausalConsole to @returns {VirtualConsole | undefined}
packages/ses/src/error/console.js
Outdated
| @@ -174,6 +174,10 @@ freeze(ErrorInfo); | |||
|
|
|||
| /** @type {MakeCausalConsole} */ | |||
| const makeCausalConsole = (baseConsole, loggedErrorHandler) => { | |||
| if (!baseConsole) { | |||
| return baseConsole; | |||
There was a problem hiding this comment.
For the sake of being sounder wrt the type I suggest above, what do you think of changing this to
| return baseConsole; | |
| return undefined; |
? Or do I miss the intent of this code?
| ? console | ||
| : typeof print === 'function' | ||
| ? // Make a good-enough console for eshost. | ||
| (p => freeze({ debug: p, log: p, info: p, warn: p, error: p }))( |
There was a problem hiding this comment.
Is a console with only these methods good enough for eshost? Is eshost likely to be the only case this code needs to accommodate?
There was a problem hiding this comment.
I believe the answer to both questions is "yes"—this is all the simple console logging functions (i.e., those that only forward their arguments to Logger with the corresponding log level), and a superset of those potentially encountered by lockdown (log, warn, and error). I've improved the comment accordingly.
gibson042
force-pushed
the
gibson-1819-lockdown-eshost
branch
from
a74f5d3 to
a2fd851
Compare
Fix #1819
Description
Tolerate the absence of non-ECMA-262 web platform APIs.
Security Considerations
This follows existing patterns and hardens all introduced new non-primitive values. It will result in calls to a global
printfunction that would not have otherwise happened, but only when it exists andconsoleis undefined (but I believe any code capable of creating those circumstances could have overriddenconsoleanyway).Scaling Considerations
n/a
Documentation Considerations
The non-web-platform environments of
eshostinvocations are sufficiently esoteric that I don't think documentation needs updating.Testing Considerations
For similar reasons, I have not introduced new tests (although I suspect that could be done).
Upgrade Considerations
None.