fix(combine): halt middleware evaluation if error in next()

[usualoma]

fixes #3898

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.28%. Comparing base (4203973) to head (033d48b).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3905   +/-   ##
=======================================
  Coverage   91.27%   91.28%           
=======================================
  Files         168      168           
  Lines       10750    10757    +7     
  Branches     3164     3165    +1     
=======================================
+ Hits         9812     9819    +7     
  Misses        937      937           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[usualoma]

Hi @satoshun00!
Thank you for the report and PR. Thanks to you, I understand the situation better.

The test code was used in the following commit.
964a1e1

I wonder if this will fix the problem.

[usualoma]

Oh, I see, we also need to handle the case where true is returned, so I've added two more commits.

• 69d6bce
• 97d0895

[satoshun00]

@usualoma
Thank you for the quick response! I was also looking at this part of the code, and I think this approach makes sense.

One small point—since some() processes multiple middleware using a for-loop, it might be worth verifying in the test code that the call count for the next function is 1.

[usualoma]

Hi @satoshun00
Thank you for your comment!
For the following reasons, I don't think it's necessary to add a test.

• If next is called multiple times, the following code will cause an error
  • https://github.com/usualoma/hono/blob/e0cf1566bb7727116683a1641a2cf914c8ef90de/src/compose.ts#L51-L53
• How next is handled depends on the internal implementation.

[usualoma]

@yusukebe
Would you please review this?

[yusukebe]

I think these lines #L61 - L62 are covered, though Codecov says they are not covered. Should you add more tests?

[usualoma]

Thanks for your comment.

The refactoring of the test in e0cf156 was not good😢 Fixed in 033d48b.

[yusukebe]

I see!

[yusukebe]

LGTM!

[yusukebe]

I see!

[yusukebe]

@usualoma Thank you for the quick fix!

@satoshun00 Thank you for raising the issue!