Skip to content

Bring the Microsoft Teams conference plugin to Zoom parity #81

Description

@Jamyn

Is your feature request related to a problem? Please describe.

The Microsoft Teams conference plugin (dispatch_microsoft_teams/conference/) is
substantially behind the Zoom plugin it parallels. Verified against main
(0fe79bda):

Teams Zoom
Methods create create, delete, add_participant, remove_participant
Metrics (counter/timer) silently absent working
Config doc none configuring-zoom.mdx
Tests none none (parity)

The metrics gap is a real defect rather than an omission. apply (src/dispatch/decorators.py:164)
is a class decorator — it iterates cls.__dict__ and rewrites each callable
attribute. Teams applies it to the create method (plugin.py:25-26), whose
__dict__ is empty, so the loop body never runs and the function is returned
untouched. Zoom applies it to the class (plugin.py:53-54) and is instrumented.
No crash, no warning — just missing telemetry.

Additional defects in dispatch_microsoft_teams/conference/client.py:

  • logger.info(f"Masked Result is {result}") (line 53) logs the entire MSAL
    token response, including access_token. Nothing is masked despite the label.
    Mitigating: the default LOG_LEVEL resolves to ERROR, so it only fires when
    an operator raises the level to INFO/DEBUG — i.e. exactly while debugging.
  • requests.post (line 65) has no timeout and can hang indefinitely.
  • No HTTP status check. A Graph error body flows into meeting_info["joinWebUrl"]
    (plugin.py:38) → KeyError → swallowed by the broad exceptcreate
    returns None.
  • recordAutomatically and isPasscodeRequired are sent as JSON strings
    ("true"/"false") rather than booleans. Inferred — not tested against live
    Graph.
  • That broad except also means the real exception never reaches
    conference/flows.py, so the incident event log records a generic "plugin
    encountered an error" (flows.py:39) instead of the cause. Zoom lets it
    propagate into the except at flows.py:28, which logs the reason.

Missing delete is not a live leak — nothing in the codebase calls
conference.delete; Zoom's is dead code too. Worth noting so it is not
prioritized as a resource leak.

Describe the solution you'd like

Bring Teams to Zoom parity:

  1. Move @apply(counter, ...)/@apply(timer, ...) from the method to the class
    so metrics actually emit.
  2. Add a timeout to the Graph call and check the HTTP status; raise on error
    rather than returning a body that fails downstream with KeyError.
  3. Stop logging the MSAL result, or log only the non-secret fields.
  4. Narrow or remove the broad except in create so the real reason reaches the
    incident event log.
  5. Send recordAutomatically/isPasscodeRequired as booleans.
  6. Add configuring-microsoft-teams.mdx alongside configuring-zoom.mdx.
  7. Implement delete/add_participant/remove_participant for interface parity
    (low priority — unused by callers).

Describe alternatives you've considered

Fixing only the metrics decorator. Rejected: the token logging and the missing
timeout are the changes with actual operator impact, and they sit in the same two
files.

Guarding against the apply-on-a-method misuse inside apply itself (raise when
handed a non-class). Reasonable as a follow-up so the mistake cannot recur
silently, but it is a separate change from making Teams work.

Additional context

Line references are against 0fe79bda. Every claim above is verified against
source except the boolean-vs-string Graph payload, which is inferred — it was
not tested against live Graph and needs a real tenant to confirm.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions