refactor: move the loginError property from Connection into Login7Handler

[arthurschreiber]

This moves the loginError property from the Connection class into Login7TokenHandler — one less piece of cross-cutting mutable state to keep track of on Connection itself.

Since a fresh Login7TokenHandler is created for each login response message (including each iteration of the NTLM challenge loop), scoping the error to the handler is naturally correct and removes the need to reset loginError between login attempts: the three performSentLogin7With* methods now read handler.loginError directly, and their finally-block cleanup of the property goes away.

This has been rebuilt on top of the current async connection establishment flow (#1661/#1663/#1665), replacing the original pre-refactor version of this change.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.37%. Comparing base (1ec67db) to head (779af1b).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/token/handler.ts	37.50%	5 Missing ⚠️
src/connection.ts	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1662      +/-   ##
==========================================
+ Coverage   80.28%   80.37%   +0.09%     
==========================================
  Files          90       90              
  Lines        4909     4908       -1     
  Branches      923      923              
==========================================
+ Hits         3941     3945       +4     
+ Misses        664      659       -5     
  Partials      304      304              

☔ 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.

[claude]

Review

Clean, well-scoped refactor. Summary of what I checked and found:

Overview

Moves loginError from Connection to Login7TokenHandler, since a fresh handler is already instantiated per login-response message (including each NTLM challenge-loop iteration). This removes the need for the finally-block this.loginError = undefined resets in the three performSentLogin7With* methods, since the error is now naturally scoped to the handler instance instead of being cross-cutting mutable state on Connection.

Correctness

• Verified all three call sites (performSentLogin7WithStandardLogin, performSentLogin7WithNTLMLogin, performSentLogin7WithFedAuth) now read handler.loginError instead of this.loginError, and the removed finally cleanup is safe because handler is declared fresh inside each attempt (and, in the NTLM case, fresh on each while (true) loop iteration), so there's no risk of a stale error leaking into a subsequent iteration/attempt.
• Confirmed there are no other references to Connection.prototype.loginError left anywhere in src/ (or elsewhere in the repo) — the removal is complete, nothing is orphaned.
• Nice catch narrowing the type from undefined | AggregateError | ConnectionError to ConnectionError | undefined on the handler — grepping through handler.ts, every assignment to loginError constructs a ConnectionError, never an AggregateError (the AggregateErrors in connection.ts are thrown directly, not stored on loginError), so the old union was overly broad. This is a small but genuine correctness/precision improvement.
• The explicit this.loginError = undefined; in the constructor is redundant given declare loginError: ConnectionError | undefined (the field starts undefined anyway), but it's harmless and arguably makes the initial state explicit next to loginAckReceived, so not a real issue.

Style/conventions

• Matches the existing pattern of colocating token-driven state (loginAckReceived, routingData, fedAuthInfoToken) on the handler rather than the connection — this change is consistent with that pattern rather than introducing a new one.

Test coverage

• No test changes included. Since this is a pure state-relocation refactor with no behavioral change (the semantics of when loginError is set/read are preserved), and existing login/NTLM/fedAuth integration tests should already exercise these paths, that seems reasonable — but worth double-checking that existing tests do cover the NTLM multi-iteration loop and the fedAuth error path, since those are exactly the cases most likely to regress if the per-handler scoping were ever subtly wrong.

Risk

Low risk. This is a mechanical, well-justified refactor that reduces cross-cutting mutable state on Connection without changing observable behavior.

🤖 Generated with Claude Code

[github-actions]

🎉 This PR is included in version 20.0.5 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀