stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

stderr-notification-handler - #149

Closed
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification
Closed

stderr-notification-handler#149
EItanya wants to merge 2 commits into
modelcontextprotocol:mainfrom
EItanya:stderr-notification

Conversation

@EItanya

@EItanyaEItanya commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Add a new notification type for stderr notification

Motivation and Context

When I was running an example using the modelcontextprotocol server-everything I ran into an issue where the library couldn't parse a new notification type:

2025-04-29T15:15:56.492628Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:15:56 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:16:06.480583Z WARN rmcp::transport::io: line: {"method":"notifications/message","params":{"level":"debug","data":"Debug-level message"},"jsonrpc":"2.0"}
2025-04-29T15:16:26.482521Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:16:26 PM: A stderr message"},"jsonrpc":"2.0"}
2025-04-29T15:17:26.503650Z WARN rmcp::transport::io: line: {"method":"notifications/stderr","params":{"content":"03:17:26 PM: A stderr message"},"jsonrpc":"2.0"}

How Has This Been Tested?

I ran this using my project which builds on this called agentgateway

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@4t145

Copy link
Copy Markdown
Contributor

Pardon me I don't find the notifications/stderr in specification, could you cite the reference?

@EItanya

Copy link
Copy Markdown
ContributorAuthor

I agree, I couldn't find it in the spec either, but it's in one of the example servers. Please see: https://github.com/modelcontextprotocol/servers/blob/de1abc85a7ddbe408fffc00f783c7e9f1a69b6b3/src/everything/everything.ts#L168.

This caused a problematic scenario because it's not in the spec, but an official example server started to use this message type which caused my clients to fail. Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream. What do you think?

@4t145

4t145 commented May 2, 2025

Copy link
Copy Markdown
Contributor

Another path we could take is having a function for unknown/arbitrary message types rather than killing the stream.

@EItanya That would be great. I think it's the proper way.

@howardjohn

Copy link
Copy Markdown
Contributor

@4t145 I was looking into this and it seems tricky to plumb through the customization semantics?

I think we would need to pass this into every transport which seems pretty heavy. I can do this but it seems pretty rough... LMK if you have other suggestions else I can do it

@4t145

4t145 commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

@howardjohn Maybe we can add an Unkown varaint for Notification and Request? I am not sure if the serde still works well after added that. If not, we may need to add some serde attibutes or manually implement deserialization.

@loocor

loocor commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

I encountered the same issue today and did some testing. @EItanya 's PR solved this problem, but this approach has some concerning tendencies:

  • Hardcoding specific types - Creating a dedicated StderrNotification type for stderr
  • Modifying core model - Adding non-standard notification types in model.rs
  • Poor extensibility - Each new non-standard notification requires adding new types
  • Deviation from standards - Formalizing non-standard content into the SDK

My suggested solution is to implement fallback handling in the decode and decode_eof methods of Decoder in crates/rmcp/src/transport/async_rw.rs, with the relevant code as follows:

/// Check if a notification method is a standard MCP notification/// should update this when MCP spec is updated about new notificationsfnis_standard_notification(method:&str) -> bool{matches!(
method,"notifications/cancelled"
| "notifications/initialized"
| "notifications/message"
| "notifications/progress"
| "notifications/prompts/list_changed"
| "notifications/resources/list_changed"
| "notifications/resources/updated"
| "notifications/roots/list_changed"
| "notifications/tools/list_changed")}/// Try to parse a message with compatibility handling for non-standard notificationsfntry_parse_with_compatibility<T: serde::de::DeserializeOwned>(line:&[u8],context:&str,) -> Result<Option<T>,JsonRpcMessageCodecError>{ifletOk(line_str) = std::str::from_utf8(line){match serde_json::from_slice(line){Ok(item) => Ok(Some(item)),Err(e) => {// Check if this is a non-standard notification that should be ignoredif line_str.contains("\"method\":\"notifications/"){// Extract the method name to check if it's standardifletOk(json_value) = serde_json::from_str::<serde_json::Value>(line_str){ifletSome(method) = json_value.get("method").and_then(|m| m.as_str()){if method.starts_with("notifications/")
&& !is_standard_notification(method){
tracing::debug!("Ignoring non-standard notification {} {}: {}",
method,
context,
line_str
);returnOk(None);// Skip this message}}}}
tracing::debug!("Failed to parse message {}: {} | Error: {}",
context,
line_str,
e
);Err(JsonRpcMessageCodecError::Serde(e))}}}else{
serde_json::from_slice(line).map(Some).map_err(JsonRpcMessageCodecError::Serde)}}```
```rustimpl<T:DeserializeOwned>DecoderforJsonRpcMessageCodec<T>{// ... ...fndecode(&mutself,buf:&mutBytesMut,) -> Result<Option<Self::Item>,JsonRpcMessageCodecError>{// ... ...(false,Some(offset)) => {// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(line,"decode")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};returnOk(Some(item));}// ... ...}}}fndecode_eof(&mutself,buf:&mutBytesMut) -> Result<Option<T>,JsonRpcMessageCodecError>{// ... ...// Use compatibility handling functionlet item = matchtry_parse_with_compatibility(&line,"decode_eof")? {Some(item) => item,None => returnOk(None),// Skip non-standard message};Some(item)}}})}}```

Sign up for freeto 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.

4 participants

@EItanya@4t145@howardjohn@loocor