Skip to content

msteams: maxRetryWait param to control waiting after 429 responses - #235

Open
benbroadaway wants to merge 3 commits into
masterfrom
bb/msteams-max-retry-wait
Open

msteams: maxRetryWait param to control waiting after 429 responses#235
benbroadaway wants to merge 3 commits into
masterfrom
bb/msteams-max-retry-wait

Conversation

@benbroadaway

Copy link
Copy Markdown
Collaborator

MS Graph API may return relatively huge durations to wait after a 429 response (e.g. 600 seconds) which may be far longer than reasonable for a process to sleep before a retry. If the response-given Retry-After is higher than the maxRetryWait param, then the task call ends with an exception (or silently ends if ignoreErrors is true).

  • Add input parameter maxRetryWait to control when to give up retrying instead of waiting for a rate limit reset
    • Default value is 30000ms (30s)
  • Mock sleep calls in tests to shave of a few seconds

@benbroadaway
benbroadaway requested a review from a team March 24, 2026 21:57
ibodrov
ibodrov previously approved these changes Apr 30, 2026

@ibodrov ibodrov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit

throw new IllegalStateException("Too many requests. Cannot wait long enough to retry.");
}

log.warn("exec [webhookUrl: '{}', params: '{}'] -> too many requests, retry after {} sec", webhookUrl, params, retryAfter);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think millisecond values as being used as seconds here: retry after 30000 sec
Also, V1 says "sec", V2 uses "ms".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astute observation. I offer a...timely...resolution 🤕

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants