Skip to content

Conversation

@erights
Copy link
Contributor

@erights erights commented Jan 13, 2025

Most PRs should close a specific Issue. All PRs should at least reference one or more Issues. Edit and/or delete the following lines as appropriate (note: you don't need both refs and closes for the same one):

Closes: #XXXX
Refs: #XXXX

Description

Add a description of the changes that this PR introduces and the files that are the most critical to review.

Security Considerations

Does this change introduce new assumptions or dependencies that, if violated, could introduce security vulnerabilities? How does this PR change the boundaries between mutually-suspicious components? What new authorities are introduced by this change, perhaps by new API calls?

Scaling Considerations

Does this change require or encourage significant increase in consumption of CPU cycles, RAM, on-chain storage, message exchanges, or other scarce resources? If so, can that be prevented or mitigated?

Documentation Considerations

Give our docs folks some hints about what needs to be described to downstream users. Backwards compatibility: what happens to existing data or deployments when this code is shipped? Do we need to instruct users to do something to upgrade their saved data? If there is no upgrade path possible, how bad will that be for users?

Testing Considerations

Every PR should of course come with tests of its own functionality. What additional tests are still needed beyond those unit tests? How does this affect CI, other test automation, or the testnet?

Compatibility Considerations

Does this change break any prior usage patterns? Does this change allow usage patterns to evolve?

Upgrade Considerations

What aspects of this PR are relevant to upgrading live production systems, and how should they be addressed?

Include *BREAKING*: in the commit message with migration instructions for any breaking change.

Update NEWS.md for user-facing changes.

Delete guidance from pull request description before merge (including this!)

@erights erights self-assigned this Jan 13, 2025
@erights erights force-pushed the markm-assertion-breakpoint branch from aee6bd2 to 1385a6c Compare January 17, 2025 19:06
* - `sanitizeError` will freeze the error, preventing any correct engine from
* adding or
* altering any of the error's own properties `sanitizeError` is done.
* altering any of the error's own properties once `sanitizeError` is done.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a driveby typo fix

@erights erights force-pushed the markm-assertion-breakpoint branch from 1385a6c to 787e0a0 Compare January 27, 2025 21:48
@erights erights force-pushed the markm-assertion-breakpoint branch from 787e0a0 to 118e02e Compare February 17, 2025 03:03
@erights erights force-pushed the markm-assertion-breakpoint branch from 118e02e to 7e38297 Compare April 8, 2025 00:57
@erights erights force-pushed the markm-assertion-breakpoint branch from 7e38297 to 39820b8 Compare May 17, 2025 01:36
@erights erights force-pushed the markm-assertion-breakpoint branch from 39820b8 to 2564f2b Compare June 3, 2025 23:33
@erights erights force-pushed the markm-assertion-breakpoint branch 2 times, most recently from 44f6330 to 8f9a3b0 Compare July 14, 2025 01:50
@erights erights force-pushed the markm-assertion-breakpoint branch from 8f9a3b0 to 3fdc99b Compare July 21, 2025 22:03
debugger;
} else if (viewAssertError === 'log') {
// eslint-disable-next-line @endo/no-polymorphic-call
console.log(error);
Copy link
Member

Choose a reason for hiding this comment

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

console.error may be more appropriate 🙂

@erights erights requested a review from Copilot September 16, 2025 08:03
Copy link
Contributor

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 adds debugging capabilities for assert failures in the SES package by introducing environment variable configuration to allow breakpoints or logging when errors are created.

  • Adds SES_VIEW_ASSERT_ERROR environment variable support with options for 'breakpoint', 'log', or 'none'
  • Implements conditional debugging behavior in the makeError function
  • Fixes a minor typo in a comment about sanitizeError

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

Comment on lines +377 to +389
const viewAssertError = getenv('SES_VIEW_ASSERT_ERROR', 'none', [
'breakpoint',
'log',
]);
if (viewAssertError !== 'none') {
if (viewAssertError === 'breakpoint') {
// eslint-disable-next-line no-debugger
debugger;
} else if (viewAssertError === 'log') {
// eslint-disable-next-line @endo/no-polymorphic-call
console.log(error);
}
}
Copy link

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

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

The environment variable is checked on every error creation. Consider caching the environment variable value at module initialization to avoid repeated calls to getenv on the error path.

Copilot uses AI. Check for mistakes.
@erights erights force-pushed the markm-assertion-breakpoint branch from 3fdc99b to 45b457f Compare September 18, 2025 04:12
@erights erights force-pushed the markm-assertion-breakpoint branch from 45b457f to f55a949 Compare September 19, 2025 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants