Skip to content

fix: raise WorkSpaceError when the depot root cannot be resolved - #17

Open
nielsvaes wants to merge 1 commit into
mainfrom
fix/depot-root-workspace-error
Open

fix: raise WorkSpaceError when the depot root cannot be resolved#17
nielsvaes wants to merge 1 commit into
mainfrom
fix/depot-root-workspace-error

Conversation

@nielsvaes

Copy link
Copy Markdown
Owner

P4Client.__init__ ended with:

self.depot_root = self.get_depot_paths([self.perforce_root])[0]

p4 where returns nothing when the client cannot talk to the server — most commonly an expired ticket — so that indexed an empty list and raised a bare IndexError: list index out of range.

The three checks immediately above it all raise WorkSpaceError naming what was missing:

if self.user is None:
    raise p4errors.WorkSpaceError("Could not find P4USER")
if self.client is None:
    raise p4errors.WorkSpaceError("Could not find P4CLIENT")
if self.server is None:
    raise p4errors.WorkSpaceError("Could not find P4PORT")

This one didn't, so a caller that reconstructs a P4Client to recover a dropped connection could not tell "log in again" apart from a genuine bug, and surfaced the IndexError text instead.

How it turned up

A Maya tool that rebuilds its P4Client on a timer to recover from ticket expiry. One session logged 7651 consecutive reconnect: failed: list index out of range warnings over just under four hours. The message gave no indication that the fix was p4 login, and it was indistinguishable from a bug in the tool.

The change

Behaviour is unchanged when the depot root resolves. When it doesn't, the constructor now raises WorkSpaceError naming the path and pointing at p4 login, instead of IndexError.

This is diagnostic only — the constructor failed before and fails now. Callers catching broad Exception see no behavioural difference; callers that catch p4errors can now distinguish an auth failure from a real defect.

Tests

Two added to tests/test_p4client.py, both verified to fail against the old code with the exact IndexError: list index out of range this fixes:

  • test_init_raises_workspace_error_when_depot_root_unresolvable
  • test_init_error_message_mentions_login

Full suite: 169 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DhkbNYwh15Ah9KLFQWXA8D

P4Client.__init__ ended with

    self.depot_root = self.get_depot_paths([self.perforce_root])[0]

`p4 where` returns nothing when the client cannot talk to the server —
most commonly an expired ticket — so that indexed an empty list and
raised a bare `IndexError: list index out of range`.

The three checks immediately above it (P4USER, P4CLIENT, P4PORT) all
raise WorkSpaceError with a message naming what was missing; this one
did not. Callers that reconstruct a P4Client to recover a dropped
connection could not tell "log in again" from a genuine bug, and logged
the IndexError text instead. A downstream tool logged 7651 of them in
one session while retrying.

Now raises WorkSpaceError naming the path and pointing at `p4 login`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant