Skip to content

httpd: improve error message for methods that requires "more"#28

Open
mvo5 wants to merge 1 commit intosystemd:mainfrom
mvo5:improve-missing-more-err
Open

httpd: improve error message for methods that requires "more"#28
mvo5 wants to merge 1 commit intosystemd:mainfrom
mvo5:improve-missing-more-err

Conversation

@mvo5
Copy link
Copy Markdown
Contributor

@mvo5 mvo5 commented Mar 25, 2026

When a method is called that requires the "more" flag in varlink we currently only error with BAD_REQUEST - however we can do better and tell the caller more deails what whent wrong. So this commit tweaks the error to return a helpful error string:

$ curl -v -X POST \
   http://localhost:1031/call/io.systemd.MachineImage.CleanPool \
   -H 'Content-Type: application/json' -d '{}'
> POST /call/io.systemd.MachineImage.CleanPool HTTP/1.1
> Content-Type: application/json
>
< HTTP/1.1 400 Bad Request
< content-type: application/json
<
{"error":"This method requires the varlink 'more' flag. Use Accept:
application/json-seq to enable streaming."}

@mvo5 mvo5 force-pushed the improve-missing-more-err branch from 984c68d to b59e3c6 Compare March 25, 2026 11:11
@keszybz
Copy link
Copy Markdown
Member

keszybz commented Apr 1, 2026

This .to_string() proliferation is ugly… Maybe like this:

diff --git src/bin/varlink-httpd/main.rs src/bin/varlink-httpd/main.rs
index e9c7af8cb6..a482b65d03 100644
--- src/bin/varlink-httpd/main.rs
+++ src/bin/varlink-httpd/main.rs
@@ -73,6 +73,7 @@ impl IntoResponse for AppError {
 impl From<zlink::Error> for AppError {
     fn from(e: zlink::Error) -> Self {
         use zlink::varlink_service;
+        let mut message = None;
         let status = match &e {
             zlink::Error::SocketRead
             | zlink::Error::SocketWrite
@@ -80,7 +81,14 @@ impl From<zlink::Error> for AppError {
             | zlink::Error::Io(..) => StatusCode::BAD_GATEWAY,
             zlink::Error::VarlinkService(owned) => match owned.inner() {
                 varlink_service::Error::InvalidParameter { .. }
-                | varlink_service::Error::ExpectedMore => StatusCode::BAD_REQUEST,
+                | varlink_service::Error::ExpectedMore => {
+                    message = Some(
+                        "This method requires the varlink 'more' flag. \
+                         Use Accept: application/json-seq to enable streaming."
+                            .to_string(),
+                    );
+                    StatusCode::BAD_REQUEST
+                }
                 varlink_service::Error::MethodNotFound { .. }
                 | varlink_service::Error::InterfaceNotFound { .. } => StatusCode::NOT_FOUND,
                 varlink_service::Error::MethodNotImplemented { .. } => StatusCode::NOT_IMPLEMENTED,
@@ -90,7 +98,7 @@ impl From<zlink::Error> for AppError {
         };
         Self {
             status,
-            message: e.to_string(),
+            message: message.unwrap_or(e.to_string()),
         }
     }
 }

?

When a method is called that requires the "more" flag in varlink
we currently only error with BAD_REQUEST - however we can do
better and tell the caller more deails what whent wrong. So this
commit tweaks the error to return a helpful error string:
```console
$ curl -v -X POST \
   http://localhost:1031/call/io.systemd.MachineImage.CleanPool \
   -H 'Content-Type: application/json' -d '{}'
> POST /call/io.systemd.MachineImage.CleanPool HTTP/1.1
> Content-Type: application/json
>
< HTTP/1.1 400 Bad Request
< content-type: application/json
<
{"error":"This method requires the varlink 'more' flag. Use Accept:
application/json-seq to enable streaming."}
```
@mvo5 mvo5 force-pushed the improve-missing-more-err branch from b59e3c6 to d9b0f93 Compare April 2, 2026 15:45
@mvo5
Copy link
Copy Markdown
Contributor Author

mvo5 commented Apr 2, 2026

Thanks @keszybz ! I love the suggestion - applied.

@mvo5 mvo5 requested a review from keszybz April 9, 2026 09:45
Sign up for free to 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.

2 participants