Skip to content

Conversation

erights
Copy link
Contributor

@erights erights commented Aug 20, 2025

Staged on #2944

Closes: #2941
Refs: #XXXX

Description

In browsers, if errorTrapping: is set to anything other than 'none', then globalWindow.addEventListener('error', event => {...});
is used to log a diagnostic that we wish contain the reason from an unhandled rejection, where the reason should be event.error.

However, in at least some browsers, event.error is absent, null, or undefined. We don't care which. In that case, all we've got is the event, so we log that instead.

Security Considerations

errorTrapping only provides a fixed menu, with the repair doing the privileged registration of a closure. This mechanism does not give post-lockdown user code any non-logging abilities it does not otherwise have. And the logging abilities are consistent with the log being write-only.

With this PR, an event object does get logged, which may contain info we would have liked to redact for normal user code. But this extra info only appears in the log, from which we never redact info anyway.

Scaling Considerations

none

Documentation Considerations

This PR does add an explanation to SES_UNCAUGHT_EXCEPTION.md, which is where it is most likely to be discovered by those who encounter this.

Testing Considerations

The relevant test were already introduced by #2944 , so this PR could illustrate the fix by the diff between it and #2944.

Compatibility Considerations

none. It is hard to imagine anything was depending on the diagnostic being inadvertently empty.

Upgrade Considerations

none

@erights erights force-pushed the markm-test-2941-error-traps branch from 37d8080 to cdca569 Compare August 20, 2025 05:45
@erights erights force-pushed the markm-fix-2941-error-traps branch from e05786a to 3b18865 Compare August 20, 2025 05:46
@erights erights self-assigned this Aug 20, 2025
@erights erights requested a review from naugtur August 20, 2025 05:48
@erights erights force-pushed the markm-test-2941-error-traps branch from cdca569 to fbb0ed1 Compare September 16, 2025 06:52
@erights erights force-pushed the markm-fix-2941-error-traps branch from 3b18865 to 4fd18f5 Compare September 16, 2025 06:57
@erights erights requested a review from Copilot September 16, 2025 08:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes error trapping issues in SES by improving error reporting when exceptions occur without catch handlers. The fix handles cases where the platform's error event may not contain an actual error object.

  • Modified error event handling to fall back to the event object itself when event.error is not available
  • Updated documentation to explain platform-specific behavior differences in error reporting

Reviewed Changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/ses/src/error/tame-console.js Adds fallback logic to report the event object when event.error is undefined
packages/ses/error-codes/SES_UNCAUGHT_EXCEPTION.md Documents platform-specific error reporting behavior and fallback mechanism

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@erights erights requested a review from gibson042 September 16, 2025 08:14
@erights erights force-pushed the markm-test-2941-error-traps branch 2 times, most recently from 86276e1 to d3972dd Compare September 18, 2025 04:32
@erights erights force-pushed the markm-fix-2941-error-traps branch from 4fd18f5 to 079a1e1 Compare September 18, 2025 04:33
@erights erights force-pushed the markm-test-2941-error-traps branch from d3972dd to 766a1d8 Compare September 19, 2025 00:41
@erights erights force-pushed the markm-fix-2941-error-traps branch from 079a1e1 to bcce95e Compare September 19, 2025 00:42
@erights erights force-pushed the markm-test-2941-error-traps branch from 766a1d8 to bdf841f Compare October 5, 2025 23:26
@erights erights force-pushed the markm-fix-2941-error-traps branch from bcce95e to cf6d174 Compare October 5, 2025 23:28
@erights erights force-pushed the markm-test-2941-error-traps branch from bdf841f to 7e11152 Compare October 7, 2025 23:46
@erights erights force-pushed the markm-fix-2941-error-traps branch from cf6d174 to e28ff91 Compare October 8, 2025 00:16
@erights erights marked this pull request as ready for review October 8, 2025 01:17
@erights
Copy link
Contributor Author

erights commented Oct 8, 2025

This is now Ready for Review

@erights erights force-pushed the markm-test-2941-error-traps branch from 9af8047 to e5da7f3 Compare October 9, 2025 17:55
@erights erights force-pushed the markm-fix-2941-error-traps branch from d189d54 to 494e5ce Compare October 9, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant