mwapi_responses: associatedpage might not exist
Open, Needs TriagePublic
Actions

Assigned To

None

Authored By

	MilkyDefer
	Nov 7 2022, 5:49 AM

Description

I tried this code.

extern crate tokio;
use mwapi_responses::prelude::*;

#[query(
    prop="info",
    inprop="associatedpage",
)]
struct Response;

#[tokio::main]
async fn main() {
    let api = mwapi::Client::builder("https://www.mediawiki.org/w/api.php").build().await.unwrap();
    let params = std::collections::HashMap::<String, String>::from_iter([
        ("action".to_string(), "query".to_string()),
        ("prop".to_string(), "info".to_string()),
        ("inprop".to_string(), "associatedpage".to_string()),
        ("titles".to_string(), "Special:FOO".to_string()),
    ]);
    let _response: Response = api.query_response(params).await.unwrap();
    println!("success");
}

The program crashed.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidJson(Error("missing field `associatedpage`", line: 0, column: 0))', src\main.rs:19:64
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The current mwapi_responses assumes that associatedpage field would always exist but there exists a corner case where if you query a special page, this field would not exist in the response.

Environment: rustc 1.64.0 | Windows 11 | mwapi 0.4.1 | mwapi_responses 0.3.1

Related Objects

Mentioned In: T354590: mwbot Bot::page_from_id() has poor error handling
Mentioned Here: T319099: mwbot-rs: Add convenience enums to mwapi_responses

Event Timeline

MilkyDefer created this task.Nov 7 2022, 5:49 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2022, 5:49 AM

I thought this would automatically add @Legoktm to the subscribers...

(I watch the project, so I should get notifications for all tasks in mwbot-rs and subprojects automatically :))

Oop. I guess we need to change the type to be Option<String>. I'll try to get to that tomorrow but anyone should feel free to beat me to it. We probably need to test other prop endpoints with special pages as well in case other things are missing like this.

I would suggest adding tests for these namespaces:

Special (ns=-1)
Media (ns=-2)
Topic (ns=2600)

The first two are special namespaces as you know, while the third one does not have an associated namespace ("Topic talk" NS2601 simply does not exist) and requires special care.

Here's the diff so far from just special pages:

diff --git a/mwapi_responses_derive/data/query+info.json b/mwapi_responses_derive/data/query+info.json
index a21bf22..6371d50 100644
--- a/mwapi_responses_derive/data/query+info.json
+++ b/mwapi_responses_derive/data/query+info.json
@@ -6,22 +6,34 @@
   "fields": [
     {
       "name": "contentmodel",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "=default"
     },
     {
       "name": "pagelanguage",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "=default"
     },
     {
       "name": "pagelanguagehtmlcode",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "=default"
     },
     {
       "name": "pagelanguagedir",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "=default"
     },
     {
@@ -66,6 +78,7 @@
         "inner": "String",
         "vec": true
       },
+      "default": true,
       "prop": "protection"
     },
     {
@@ -74,6 +87,7 @@
         "inner": "mwapi_responses::protection::ProtectionInfo",
         "vec": true
       },
+      "default": true,
       "prop": "protection"
     },
     {
@@ -86,7 +100,10 @@
     },
     {
       "name": "watched",
-      "type_": "bool",
+      "type_": {
+        "inner": "bool",
+        "option": true
+      },
       "prop": "watched"
     },
     {
@@ -124,22 +141,34 @@
     },
     {
       "name": "associatedpage",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "associatedpage"
     },
     {
       "name": "fullurl",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "url"
     },
     {
       "name": "editurl",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "url"
     },
     {
       "name": "canonicalurl",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "url"
     },
     {
@@ -152,7 +181,10 @@
     },
     {
       "name": "displaytitle",
-      "type_": "String",
+      "type_": {
+        "inner": "String",
+        "option": true
+      },
       "prop": "displaytitle"
     },
     {
@@ -177,10 +209,10 @@
   "test_extra": {
     "assert": {
       "continue": false,
-      "length": 3
+      "length": 4
     },
     "params": {
-      "titles": "Taylor Swift|Talk:Taylor Swift|This article does not exist, please"
+      "titles": "Taylor Swift|Talk:Taylor Swift|This article does not exist, please|Special:BlankPage"
     }
   }
 }

I think it's probably worth working on the enums suggestion sooner rather than later from T319099: mwbot-rs: Add convenience enums to mwapi_responses to make this less painful for the most common case (non-special pages).

Legoktm mentioned this in T354590: mwbot Bot::page_from_id() has poor error handling.Jan 9 2024, 6:06 AM

mwapi_responses: associatedpage might not existOpen, Needs TriagePublicActions

Description

Related Objects

Event Timeline

mwapi_responses: associatedpage might not exist
Open, Needs TriagePublic
Actions