Skip to content

Conversation

@logaretm
Copy link
Collaborator

@logaretm logaretm commented Dec 3, 2025

This is the opposite of the flake that happened in #18035, so apparently nuxt-3 flaky tests started after merging that PR.

It looks like the same problem, we have a race condition, if Nitro catches the error first when executing the middleware handler it will be swallowed by an H3 error but it seems the instanceof check isn't being satisfied in this case, so perhaps with Nuxt 3, there isn't a unified H3 instance or perhaps we are using a different one for the check.

I will take a better look later, this is a band-aid to stop this from randomly failing our PRs. I couldn't reproduce this locally at all tho.

If we don't like this relaxation we can close this PR and I can spend more time on this, I thought the most important thing here is we do capture an error with the critical information.

@logaretm logaretm marked this pull request as ready for review December 3, 2025 16:54
@logaretm logaretm requested review from Copilot, isaacs and s1gr1d December 3, 2025 16:54
Copy link

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 relaxes a flaky test assertion in the Nuxt 3 middleware error handling test. The change addresses a race condition where either Sentry or Nitro may capture unhandled errors first, resulting in different mechanism types ('auto.middleware.nuxt' vs 'chained').

  • Commented out the strict mechanism type assertion to prevent random test failures
  • Added explanatory comments documenting the race condition behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 124 to 128
// Type changes depending on whether it is being wrapped by Nitro or not
// This is a timing problem, sometimes Nitro can capture the error first, and sometimes it can't
// If nitro captures the error first, the type will be 'chained'
// If Sentry captures the error first, the type will be 'auto.middleware.nuxt'
// type: 'auto.middleware.nuxt',
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The comment explains that the mechanism type changes based on race conditions, but the assertion no longer validates the mechanism type at all. Consider asserting that the type is either 'auto.middleware.nuxt' or 'chained' to maintain some test coverage:

type: expect.stringMatching(/^(auto\.middleware\.nuxt|chained)$/),

This would document the expected behavior while handling the race condition.

Copilot uses AI. Check for mistakes.
@logaretm logaretm merged commit 862f415 into develop Dec 4, 2025
35 checks passed
@logaretm logaretm deleted the awad/nuxt-3-relaxed-error-assertion branch December 4, 2025 11:16
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.

3 participants