Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly assert on badged errors instead of ignoring badged errors @eps1lon #6

Open
wants to merge 10 commits into
base: errorenv
Choose a base branch
from

Conversation

eps1lon
Copy link

@eps1lon eps1lon commented Jun 19, 2024

No description provided.

sebmarkbage and others added 10 commits June 12, 2024 10:56
This is attached to our custom Error "sub class" on the receiving side
which also includes digest.

DEV-only.
This way this helper can use component stacks when available.
That way your typical log of an error that originated on the Server gets
a `[Server]` badge.

Unfortunately because defaultOnUncaughtError and defaultOnRecoverableError
doesn't use custom logging but goes through reportError. There's no place
for us to add this kind of formatted badging to the logs.

It's also unfortunate that you'd have to replicate this in user space.
@@ -841,6 +841,10 @@ describe('ReactFlightDOM', () => {
expectedGamesValue,
);

if (gate(flags => flags.enableOwnerStacks)) {
Copy link
Author

Choose a reason for hiding this comment

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

I was expecting enableServerComponentLogs but the behavior is not actually behind this flag. Using enableServerComponentLogs would fail CI.

@@ -841,6 +841,10 @@ describe('ReactFlightDOM', () => {
expectedGamesValue,
);

if (gate(flags => flags.enableOwnerStacks)) {
// TODO: assertConsoleServerErrorDev
Copy link
Author

Choose a reason for hiding this comment

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

Probably best to use a dedicated assertConsoleBadgedErrorDev matcher with the badge name. That way we could also test switching environment names halfway through.

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.

2 participants