massive-com / massive-com/client-python

Inconsistent typing for WebSocketMessage and WebSocketClient.run() callback in v2.8.0

Open
#1,022 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
1.5k
Forks
362
PR merge metrics
No merged PRs in 30d

Description

Hi Massive team,

I noticed an inconsistency in the websocket typing in Massive Python SDK v2.8.0.

WebSocketMessage is currently defined as a NewType over a list of parsed event models:

WebSocketMessage = NewType(
    "WebSocketMessage",
    List[
        Union[
            EquityAgg,
            CurrencyAgg,
            EquityTrade,
            ...
        ]
    ],
)

However, WebSocketClient.run() is typed as accepting a callback of:

Callable[[List[WebSocketMessage]], None]

This effectively makes the callback parameter type a nested list-like structure:
List[WebSocketMessage], while WebSocketMessage itself already represents a list
of parsed websocket events.

This causes type checkers such as Pyright/Pylance to reject handlers like this:

def handle_msg(messages: WebSocketMessage) -> None:
    for message in messages:
        if isinstance(message, EquityAgg):
            ...

with an error similar to:

Argument of type "(messages: WebSocketMessage) -> None" cannot be assigned to parameter "handle_msg"
Type "(messages: WebSocketMessage) -> None" is not assignable to type "(List[WebSocketMessage]) -> None"

There also seems to be a related inconsistency in the parser:

def parse_single(...) -> Optional[WebSocketMessage]:
    parsed = model_class.from_dict(data)
    return cast(WebSocketMessage, parsed)

At runtime, parsed is a single event object such as EquityAgg, not a list. Then
parse() returns List[WebSocketMessage].

So there appear to be two possible fixes:

  1. If WebSocketMessage is intended to mean a single parsed event, redefine it as
    a union of event model types, not a list.
  2. If WebSocketMessage is intended to mean a batch of parsed events, then run()
    should accept Callable[[WebSocketMessage], None], and parse_single() should
    not cast individual event objects to WebSocketMessage.

Based on the current runtime behavior, option 1 seems more natural:

WebSocketMessage = NewType(
    "WebSocketMessage",
    Union[
        EquityAgg,
        CurrencyAgg,
        EquityTrade,
        ...
    ],
)

or alternatively, using a type alias instead of NewType:

WebSocketMessage = Union[
    EquityAgg,
    CurrencyAgg,
    EquityTrade,
    ...
]

Then these signatures would make sense:

def parse_single(...) -> Optional[WebSocketMessage]: ...
def parse(...) -> List[WebSocketMessage]: ...

def run(
    self,
    handle_msg: Callable[[List[WebSocketMessage]], None] | Callable[[str | bytes], None],
    close_timeout: int = 1,
    **kwargs: Any,
) -> None: ...

One smaller typing improvement: run() currently leaves **kwargs unknown for
Pyright/Pylance. Annotating it as **kwargs: Any and adding -> None would avoid
"partially unknown" warnings.

Thanks.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the definitions of WebSocketMessage, WebSocketClient.run(), parse_single(), and parse() mentioned in the issue, then inspect their runtime values and type signatures. Confirm whether WebSocketMessage represents one parsed event or a batch, align the callback and parser annotations accordingly, and include the proposed annotations for run() so Pyright/Pylance accepts a correctly typed handler.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.