[kapunakap]

What

HermesPromiseHandler.RequestPromise reports failures by sending an error on its result channel and reports success by closing that channel without sending a value.

InvoiceTracker.handlePromiseErrors currently forwards only values received from that channel. As a result, a successful promise request never reaches handleHermesError(nil), so hermesFailureCount is not reset after success. Retryable Hermes failures can therefore accumulate across successful promise requests even though the configured limit is intended to cover failures "in a row".

This change treats a clean result-channel close as a successful completion and forwards nil to the existing Hermes error handler. If the channel emitted any result, closing it does not synthesize an additional success result.

The forwarding is also stop-aware so a late result cannot block the helper after the invoice tracker has stopped.

Tests

Added regression coverage that verifies:

• a clean promise-result channel close reports successful completion;
• an error followed by channel close does not produce a false success;
• successful completion resets an existing Hermes failure count.

Related to #6218. This is a narrow payment-engine correctness fix; it does not claim to resolve all connection churn described there.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kapunakap]

Hi @Waldz — this PR is still relevant and addresses the Hermes consecutive-failure counter issue described in #6218.

The fix is narrowly scoped and includes regression tests. The Tests workflow is currently blocked with action_required and no jobs have started.

Could a maintainer please approve GitHub Actions and review the change? Happy to address any CI failures or review feedback.

Thanks!

[mdomasevicius]

@kapunakap will need some more time to review, @Waldz has moved to other matters

[kapunakap]

I gotchu... this is too slow, as you surely know. Are you guys hiring?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.16%. Comparing base (cf3f5aa) to head (5156c47).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
session/pingpong/invoice_tracker.go	70.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6219      +/-   ##
==========================================
+ Coverage   26.00%   26.16%   +0.15%     
==========================================
  Files         544      544              
  Lines       31565    31597      +32     
==========================================
+ Hits         8210     8266      +56     
+ Misses      22509    22481      -28     
- Partials      846      850       +4     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mdomasevicius]

@kapunakap I have to push to origin from your fork because CI workflow pipeline doesn't share secrets when running fork PRs

6226

[mdomasevicius]

Merged with #6226