Skip to content

handler/session: add Validate() to catch nil Sessions and URLParamFunc at startup#289

Merged
veverkap merged 1 commit into
mainfrom
copilot/add-sessionhandler-validate-method
May 21, 2026
Merged

handler/session: add Validate() to catch nil Sessions and URLParamFunc at startup#289
veverkap merged 1 commit into
mainfrom
copilot/add-sessionhandler-validate-method

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

SessionHandler was the only handler in the package without a Validate() method, meaning nil Sessions or URLParamFunc fields caused a runtime panic instead of a startup error.

Changes

  • handler/session.go — adds Validate() error following the established handler pattern; returns a descriptive error if Sessions or URLParamFunc is nil
  • handler/session_test.go — adds three cases: nil Sessions, nil URLParamFunc, and a fully configured handler
func (h *SessionHandler) Validate() error {
    if h.Sessions == nil {
        return errors.New("SessionHandler misconfigured: Sessions is required")
    }
    if h.URLParamFunc == nil {
        return errors.New("SessionHandler misconfigured: URLParamFunc is required")
    }
    return nil
}

Greptile Summary

This PR adds a Validate() method to SessionHandler, catching nil Sessions and URLParamFunc fields at startup rather than allowing a nil-dereference panic at the first request. The implementation and error messages match the pattern used by every other handler in the package.

  • handler/session.go: Validate() checks Sessions then URLParamFunc for nil, returning a descriptive error for each; no other logic is changed.
  • handler/session_test.go: Three new test cases cover nil Sessions, nil URLParamFunc, and a fully configured handler, consistent with existing test style.

Confidence Score: 5/5

Safe to merge — the change is purely additive and touches no existing request-handling logic.

Both changed files are small and self-contained: a two-condition guard method and three matching tests. The implementation mirrors the pattern established by every other handler in the package, existing slog calls already pass context correctly, and no request paths are modified.

No files require special attention.

Important Files Changed

Filename Overview
handler/session.go Adds Validate() to SessionHandler following the established pattern; checks Sessions and URLParamFunc for nil with descriptive error messages. No issues found.
handler/session_test.go Adds three focused test cases covering nil Sessions, nil URLParamFunc, and a fully configured handler. Tests are correct and consistent with the existing test style in the package.

Sequence Diagram

sequenceDiagram
    participant App as Application Startup
    participant SH as SessionHandler
    participant Store as SessionStore
    participant UPF as URLParamFunc

    App->>SH: Validate()
    SH->>Store: "Sessions == nil?"
    alt Sessions is nil
        SH-->>App: error: Sessions is required
    else Sessions is set
        SH->>UPF: "URLParamFunc == nil?"
        alt URLParamFunc is nil
            SH-->>App: error: URLParamFunc is required
        else URLParamFunc is set
            SH-->>App: nil (valid)
        end
    end
Loading

Reviews (1): Last reviewed commit: "Add Validate() to SessionHandler with ni..." | Re-trigger Greptile

@veverkap veverkap marked this pull request as ready for review May 20, 2026 14:44
@veverkap veverkap requested review from a team and Copilot May 20, 2026 14:44
Copy link
Copy Markdown
Contributor

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@veverkap veverkap requested a review from Copilot May 21, 2026 01:13
@veverkap veverkap merged commit fc02efb into main May 21, 2026
21 checks passed
@veverkap veverkap deleted the copilot/add-sessionhandler-validate-method branch May 21, 2026 01:13
Copy link
Copy Markdown
Contributor

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.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

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