make onprogress
handler optional for progress flow
#262
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The protocol implementation indirectly mandates an
onprogress
handler to be set while making a request to a Server which is expected to send Progress notifications.The behaviour requirements in the protocol don't seem to mandate this
onprogress
callback in theSender
. Hence, IMOonprogress
handler should be optional. In case client is interested in logging/processing the progress updates) they can set it otherwise they should not be required to pass in theonprogress
callback.This was discovered while working on modelcontextprotocol/inspector#271.
To demonstrate further
Currently in order for progress notification based timeout resets to work, this is the minimum setup:
^ Here
onprogress
is mandatory due to the way_onprogress
hook is implemented in the protocol, it checks for presence of aprogressHandler
which is only set when usingonprogress
Post this PR, the following would work (added test for the same):
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context