JMAP incompatible with Stalwart server #192

New Issue

rudihorn · 2022-10-02T22:05:26+03:00

rudihorn commented

2022-10-02 22:05:26 +03:00

A new JMAP compatible mail server called Stalwart has recently been made public, but it seems to not be compatible with the meli JMAP implementation.

When I start meli it just says "loading... : JobRequest::Watch".

The server is very easy to set up and test, and seems to be one of the few open source mail servers taking JMAP seriously, so support would be great.

A new JMAP compatible mail server called Stalwart has recently been made public, but it seems to not be compatible with the meli JMAP implementation. When I start meli it just says "loading... <guid>: JobRequest::Watch". The server is very easy to set up and test, and seems to be one of the few open source mail servers taking JMAP seriously, so support would be great.

Manos Pitsidianakis commented

2022-10-03 20:46:53 +03:00

Thanks heaps for the error report! I had time to setup stalwart today and do some investigating.

This behavior is a combination of many bugs (doh):

Optionally if running a server without ssl: connection hostname string isn't handled correctly and defaults to SSL
returning connection errors on jmap doesn't set ErrorKind which doesn't alert the frontend that it should stop retrying; this is just bad design in the code
stalwart appears to default all mailboxes to not subscribed

The last one was very unexpected. The spec https://jmap.io/spec-mail.html#mailboxes says this is allowed if the account is marked as personal, which stalwart does but meli does not check.

I'll push a stream of fixes in the next few days.

Thanks heaps for the error report! I had time to setup stalwart today and do some investigating. This behavior is a combination of many bugs (doh): - Optionally if running a server without ssl: connection hostname string isn't handled correctly and defaults to SSL - returning connection errors on jmap doesn't set `ErrorKind` which doesn't alert the frontend that it should stop retrying; this is just bad design in the code - stalwart appears to default all mailboxes to not subscribed The last one was very unexpected. The spec https://jmap.io/spec-mail.html#mailboxes says this is allowed if the account is marked as personal, which stalwart does but `meli` does not check. I'll push a stream of fixes in the next few days.

Manos Pitsidianakis added the

JMAP

bug

labels 2022-10-03 20:47:03 +03:00

Manos Pitsidianakis referenced this issue from a commit

2022-10-04 15:59:22 +03:00

melib/jmap: mark mailboxes as subscribed on personal accounts

Manos Pitsidianakis closed this issue

2022-10-04 15:59:22 +03:00

Manos Pitsidianakis commented

2022-10-04 16:00:06 +03:00

Looks like I fixed this specific issue but there's still work for painless JMAP use in general.

rudihorn commented

2022-10-04 22:59:11 +03:00

Poster

Thank you very much for working on this! Your issue definitely fixes the loading JobRequest::Watch message, however for me it still says offline: Account is uninitialized. I'll await your further patches and see if they fix the issue though.

Thank you very much for working on this! Your issue definitely fixes the loading JobRequest::Watch message, however for me it still says `offline: Account is uninitialized`. I'll await your further patches and see if they fix the issue though.

Manos Pitsidianakis commented

2022-10-05 09:52:07 +03:00

@rudihorn I will add some more error reporting on the JMAP connection code and ping you here when I push it.

Manos Pitsidianakis commented

2022-10-08 15:18:15 +03:00

@rudihorn hey, I added some jmap error prints in jmap-status-and-connect-retry-wip branch. Could you see if it shows any readable error? Otherwise, if you are comfortable with applying a tiny patch that adds some debug prints to stderr, can you apply this (copy text, save as name.patch, and in the root directory of the git repository run git apply path/to/name.patch)

To run with debug prints on, build and run like this:

cargo build --features=debug-tracing
cargo run --features=debug-tracing 2> meli-stderr.log

which will redirect the standard error output stream (stderr) to the file meli-stderr.log. Otherwise it will all get printed in the terminal and mess the display up.

diff --git a/Cargo.toml b/Cargo.toml
index 3a42e116..33814122 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -81,7 +81,7 @@ strip = true
 members = ["melib", "tools", ]
 
 [features]
-default = ["sqlite3", "notmuch", "regexp", "smtp", "dbus-notifications", "gpgme", "cli-docs"]
+default = ["sqlite3", "notmuch", "regexp", "smtp", "dbus-notifications", "gpgme", "cli-docs", "jmap"]
 notmuch = ["melib/notmuch_backend", ]
 jmap = ["melib/jmap_backend",]
 sqlite3 = ["melib/sqlite3"]
diff --git a/melib/src/backends/jmap/connection.rs b/melib/src/backends/jmap/connection.rs
index 1cff3a95..a2f965ac 100644
--- a/melib/src/backends/jmap/connection.rs
+++ b/melib/src/backends/jmap/connection.rs
@@ -110,6 +110,7 @@ impl JmapConnection {
         }
 
         *self.store.online_status.lock().await = (Instant::now(), Ok(()));
+        debug!("session {:#?}", &session);
         *self.session.lock().unwrap() = session;
         Ok(())
     }
diff --git a/melib/src/backends/jmap/protocol.rs b/melib/src/backends/jmap/protocol.rs
index a8bd2ff4..598d5099 100644
--- a/melib/src/backends/jmap/protocol.rs
+++ b/melib/src/backends/jmap/protocol.rs
@@ -102,6 +102,7 @@ pub async fn get_mailboxes(conn: &JmapConnection) -> Result<HashMap<MailboxHash,
         .await?;
 
     let res_text = res.text().await?;
+    debug!("get_mailboxes response {:?}", &res_text);
     let mut v: MethodResponse = serde_json::from_str(&res_text).unwrap();
     *conn.store.online_status.lock().await = (std::time::Instant::now(), Ok(()));
     let m = GetResponse::<MailboxObject>::try_from(v.method_responses.remove(0))?;
@@ -268,12 +269,14 @@ pub async fn fetch(
     req.add_call(&email_call);
 
     let api_url = conn.session.lock().unwrap().api_url.clone();
+    debug!("post req {:?}", &email_call);
     let mut res = conn
         .client
-        .post_async(api_url.as_str(), serde_json::to_string(&req)?)
+        .post_async(api_url.as_str(), debug!(serde_json::to_string(&req))?)
         .await?;
 
     let res_text = res.text().await?;
+    debug!("first post response {:?}", &res_text);
 
     let mut v: MethodResponse = serde_json::from_str(&res_text).unwrap();
     let e = GetResponse::<EmailObject>::try_from(v.method_responses.pop().unwrap())?;
diff --git a/src/conf/accounts.rs b/src/conf/accounts.rs
index 8344840b..425ea810 100644
--- a/src/conf/accounts.rs
+++ b/src/conf/accounts.rs
@@ -1642,7 +1642,7 @@ impl Account {
         if let Some(mut job) = self.active_jobs.remove(job_id) {
             match job {
                 JobRequest::Mailboxes { ref mut handle } => {
-                    if let Ok(Some(mailboxes)) = handle.chan.try_recv() {
+                    if let Ok(Some(mailboxes)) = debug!(handle.chan.try_recv()) {
                         if let Err(err) = mailboxes.and_then(|mailboxes| self.init(mailboxes)) {
                             if err.kind.is_authentication() {
                                 self.sender

@rudihorn hey, I added some jmap error prints in [`jmap-status-and-connect-retry-wip`](https://git.meli.delivery/meli/meli/src/branch/jmap-status-and-connect-retry-wip) branch. Could you see if it shows any readable error? Otherwise, if you are comfortable with applying a tiny patch that adds some debug prints to stderr, can you apply this (copy text, save as `name.patch`, and in the root directory of the git repository run `git apply path/to/name.patch`) To run with debug prints on, build and run like this: ```cli cargo build --features=debug-tracing cargo run --features=debug-tracing 2> meli-stderr.log ``` which will redirect the standard error output stream (`stderr`) to the file `meli-stderr.log`. Otherwise it will all get printed in the terminal and mess the display up. ```diff diff --git a/Cargo.toml b/Cargo.toml index 3a42e116..33814122 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,7 +81,7 @@ strip = true members = ["melib", "tools", ] [features] -default = ["sqlite3", "notmuch", "regexp", "smtp", "dbus-notifications", "gpgme", "cli-docs"] +default = ["sqlite3", "notmuch", "regexp", "smtp", "dbus-notifications", "gpgme", "cli-docs", "jmap"] notmuch = ["melib/notmuch_backend", ] jmap = ["melib/jmap_backend",] sqlite3 = ["melib/sqlite3"] diff --git a/melib/src/backends/jmap/connection.rs b/melib/src/backends/jmap/connection.rs index 1cff3a95..a2f965ac 100644 --- a/melib/src/backends/jmap/connection.rs +++ b/melib/src/backends/jmap/connection.rs @@ -110,6 +110,7 @@ impl JmapConnection { } *self.store.online_status.lock().await = (Instant::now(), Ok(())); + debug!("session {:#?}", &session); *self.session.lock().unwrap() = session; Ok(()) } diff --git a/melib/src/backends/jmap/protocol.rs b/melib/src/backends/jmap/protocol.rs index a8bd2ff4..598d5099 100644 --- a/melib/src/backends/jmap/protocol.rs +++ b/melib/src/backends/jmap/protocol.rs @@ -102,6 +102,7 @@ pub async fn get_mailboxes(conn: &JmapConnection) -> Result<HashMap<MailboxHash, .await?; let res_text = res.text().await?; + debug!("get_mailboxes response {:?}", &res_text); let mut v: MethodResponse = serde_json::from_str(&res_text).unwrap(); *conn.store.online_status.lock().await = (std::time::Instant::now(), Ok(())); let m = GetResponse::<MailboxObject>::try_from(v.method_responses.remove(0))?; @@ -268,12 +269,14 @@ pub async fn fetch( req.add_call(&email_call); let api_url = conn.session.lock().unwrap().api_url.clone(); + debug!("post req {:?}", &email_call); let mut res = conn .client - .post_async(api_url.as_str(), serde_json::to_string(&req)?) + .post_async(api_url.as_str(), debug!(serde_json::to_string(&req))?) .await?; let res_text = res.text().await?; + debug!("first post response {:?}", &res_text); let mut v: MethodResponse = serde_json::from_str(&res_text).unwrap(); let e = GetResponse::<EmailObject>::try_from(v.method_responses.pop().unwrap())?; diff --git a/src/conf/accounts.rs b/src/conf/accounts.rs index 8344840b..425ea810 100644 --- a/src/conf/accounts.rs +++ b/src/conf/accounts.rs @@ -1642,7 +1642,7 @@ impl Account { if let Some(mut job) = self.active_jobs.remove(job_id) { match job { JobRequest::Mailboxes { ref mut handle } => { - if let Ok(Some(mailboxes)) = handle.chan.try_recv() { + if let Ok(Some(mailboxes)) = debug!(handle.chan.try_recv()) { if let Err(err) = mailboxes.and_then(|mailboxes| self.init(mailboxes)) { if err.kind.is_authentication() { self.sender ```

rudihorn commented

2022-10-09 19:04:45 +03:00

Poster

Hey @epilys I'm more than happy to do this as soon as I am back from Holiday. Will send you this some time later this week.

Manos Pitsidianakis commented

2022-10-09 20:20:42 +03:00

Good, no problem! Until then, if anyone else is facing the same issue, can you post details about the stalwart setup? (port, using tls y/n) Because in my default stalwart setup I can connect to it.

rudihorn commented

2022-10-12 21:51:15 +03:00

Poster

@epilys

Okay so I did a little bit of testing, the first issue seems to be that the meli code tries to construct a URI, but is missing a scheme. Setting the server_hostname to https://<url> fixes this issue. It would be nice if the server_hostname could be made completely optional though. Ideally a better solution might be to introduce an optional server_url and then remove the server_hostname and server_port options.

This somewhat works and shows up the mailboxes and allows me to somewhat browse through mail. There is however an error message missing field 'accountId' at line 1 column 178. I'm also facing issues where if I try scrolling down messages, it goes back up to the first message.

Edit: the accountId error seems to only be for some mailboxes, possibly larger ones.

@epilys Okay so I did a little bit of testing, the first issue seems to be that the meli code tries to construct a URI, but is missing a scheme. Setting the `server_hostname` to `https://<url>` fixes this issue. It would be nice if the `server_hostname` could be made completely optional though. Ideally a better solution might be to introduce an optional `server_url` and then remove the `server_hostname` and `server_port` options. This somewhat works and shows up the mailboxes and allows me to somewhat browse through mail. There is however an error message `missing field 'accountId' at line 1 column 178`. I'm also facing issues where if I try scrolling down messages, it goes back up to the first message. Edit: the accountId error seems to only be for some mailboxes, possibly larger ones.

Manos Pitsidianakis commented

2022-10-13 10:29:34 +03:00

@epilys

Okay so I did a little bit of testing, the first issue seems to be that the meli code tries to construct a URI, but is missing a scheme. Setting the server_hostname to https://<url> fixes this issue. It would be nice if the server_hostname could be made completely optional though. Ideally a better solution might be to introduce an optional server_url and then remove the server_hostname and server_port options.

That makes complete sense! I copied the configuration flags from IMAP when I started implementing JMAP, and it was a mistake. I will make the change.

This somewhat works and shows up the mailboxes and allows me to somewhat browse through mail. There is however an error message missing field 'accountId' at line 1 column 178. I'm also facing issues where if I try scrolling down messages, it goes back up to the first message.

Edit: the accountId error seems to only be for some mailboxes, possibly larger ones.

Ok, as a next step I will try to return a more helpful error when JSON deserialization fails from the response of the server. I will also try to setup a stalwart instance and copy a bunch of mail in so I'm not testing it empty.

> @epilys > > Okay so I did a little bit of testing, the first issue seems to be that the meli code tries to construct a URI, but is missing a scheme. Setting the `server_hostname` to `https://<url>` fixes this issue. It would be nice if the `server_hostname` could be made completely optional though. Ideally a better solution might be to introduce an optional `server_url` and then remove the `server_hostname` and `server_port` options. That makes complete sense! I copied the configuration flags from IMAP when I started implementing JMAP, and it was a mistake. I will make the change. > This somewhat works and shows up the mailboxes and allows me to somewhat browse through mail. There is however an error message `missing field 'accountId' at line 1 column 178`. I'm also facing issues where if I try scrolling down messages, it goes back up to the first message. > > Edit: the accountId error seems to only be for some mailboxes, possibly larger ones. Ok, as a next step I will try to return a more helpful error when JSON deserialization fails from the response of the server. I will also try to setup a stalwart instance and copy a bunch of mail in so I'm not testing it empty.

Manos Pitsidianakis commented

2022-10-13 11:02:04 +03:00

@rudihorn pushed in 0ef4dde9 55ed9624

If you have the time to check it out again... In the meantime I will try to reproduce this. 🙃

@rudihorn pushed in 0ef4dde9 55ed9624 If you have the time to check it out again... In the meantime I will try to reproduce this. 🙃

rudihorn commented

2022-10-13 20:39:33 +03:00

Poster

The server_url field now works great and my suspicion of the large mailbox was correct. When trying to open larger mailboxes I get the error:

could not fetch mailbox BUG: Could not deserialize server JSON response properly, please report this!
Reply from server: ["error",{"type":"requestTooLarge","description":"The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call."},"m2"]

The scrolling resetting itself seems to have gone away, but I will still keep an eye on this to see if it comes back. When I open an email it still only shows the header and no body. There is no error message. I can maybe see if I can debug what is going on there unless you have any specific ideas of what it could be?

The mail server provides its limitations, both in the capabilities section as well as for each account e.g.:

{"urn:ietf:params:jmap:core":{"maxSizeUpload":50000000,"maxConcurrentUpload":4,"maxSizeRequest":10000000,"maxConcurrentRequests":4,"maxCallsInRequest":16,"maxObjectsInGet":500,"maxObjectsInSet":500,"collationAlgorithms":["i;ascii-numeric","i;ascii-casemap","i;unicode-casemap"]}

The `server_url` field now works great and my suspicion of the large mailbox was correct. When trying to open larger mailboxes I get the error: ``` could not fetch mailbox BUG: Could not deserialize server JSON response properly, please report this! Reply from server: ["error",{"type":"requestTooLarge","description":"The number of ids requested by the client exceeds the maximum number the server is willing to process in a single method call."},"m2"] ``` The scrolling resetting itself seems to have gone away, but I will still keep an eye on this to see if it comes back. When I open an email it still only shows the header and no body. There is no error message. I can maybe see if I can debug what is going on there unless you have any specific ideas of what it could be? The mail server provides its limitations, both in the capabilities section as well as for each account e.g.: ``` {"urn:ietf:params:jmap:core":{"maxSizeUpload":50000000,"maxConcurrentUpload":4,"maxSizeRequest":10000000,"maxConcurrentRequests":4,"maxCallsInRequest":16,"maxObjectsInGet":500,"maxObjectsInSet":500,"collationAlgorithms":["i;ascii-numeric","i;ascii-casemap","i;unicode-casemap"]} ```

Manos Pitsidianakis commented

2022-10-14 15:33:09 +03:00

Nice! The mailbox fetch request size should have been updated when I converted them to use batches instead of fetching all emails. JMAP was unused and unupdated. I will push a fix on this soon.

Thanks a lot for your help and patience @rudihorn

When I open an email it still only shows the header and no body. There is no error message. I can maybe see if I can debug what is going on there unless you have any specific ideas of what it could be?

Sounds like the request to get the email never completes for some reason.

Nice! The mailbox fetch request size should have been updated when I converted them to use batches instead of fetching all emails. JMAP was unused and unupdated. I will push a fix on this soon. Thanks a lot for your help and patience @rudihorn > When I open an email it still only shows the header and no body. There is no error message. I can maybe see if I can debug what is going on there unless you have any specific ideas of what it could be? Sounds like the request to get the email never completes for some reason.

Manos Pitsidianakis commented

2022-10-15 18:13:18 +03:00

WIP here, untested properly yet

#158

WIP here, untested properly yet #158

Sign in to join this conversation.