"code:badinteger" and "code:missingtitle" errors even on successful uploads

[mnalis]

Feedback

Even in cases where upload of the picture seems to work OK, Commons app logs dozens of API errors in format:

• "code":"badinteger","info":"Invalid value "M133255861" for integer parameter "pageids". and
• "code":"missingtitle","info":"The page you specified doesn't exist."

with ERROR priority. If those are expected, they should likely be DEBUG (hopefully with few extra words of explanation).

If those are not expected, they should probably be looked into why they are happening, and what are sideeffects / possible problems caused by them.

Here is example video of successful upload:

small_SVID_20230620_190215_1.mp4

And here are logs with lots of mentioned errors: CommonsAppLogs.0.log

Wiki username

Mnalis

Device name

Huawei P30Pro

Android version

Android 10 (EMUI 12)

Commons app version

4.1.0~1649d1e2c

[psh]

The specific logs ("code:badinteger" and "code:missingtitle") come from code that doesnt care about them being errors because they either return empty values or, fall back to a second method to load data

• in the case of the "badinteger" we are kicking off a couple of requests (to getMediaById() and getMedia()) in the MediaDataExtractor
• and the missingtitle comes from a call to getPageHtml() that returns empty strings

Currently we log all errors reported by the backend API in our UnsuccessfulResponseInterceptor at error level. Given that this is an OkHttp Interceptor it has no knowledge of the Retrofit interface or the code calling into the retrofit interface. The interceptor should remain decoupled, so I would suggest adding a header at the retrofit level - for example

    @GET("w/api.php?format=json&action=parse&prop=text")
    @Headers("x-commons-suppress-error-log: true")
    fun getPageHtml(@Query("page") title: String?): Single<MwParseResponse>

which in the interceptor we can make sure never gets sent to the server, but acts as a trigger to suppress problems

    public Response intercept(@NonNull final Chain chain) throws IOException {
        final Request rq = chain.request();
        
        // If the request contains our special "suppress errors" header, dont pass that
        // on to the server.
        final boolean suppressErrors = rq.headers().names().contains("x-commons-suppress-error-log");
        final Request request = rq.newBuilder()
            .removeHeader("x-commons-suppress-error-log")
            .build();

        final Response rsp = chain.proceed(request);
 
        .
        .
        .

and if a problem occurs,

        .
        .
        .
    } catch (final IOException e) {
        // Depending on the annotated interface, log the error as debug
        //  (and therefore, "expected") or at error level.
        if (suppressErrors) {
            Timber.d(e, "Suppressed (known / expected) error");
        } else {
            Timber.e(e);
        }
    }

The use of a header feels a little strange, but I don't see a neat per-request way to tag a given retrofit call with meta data that the interceptor can use; it only knows about requests and responses and the solution gets beyond the complexity that suppressing a couple of log statements really warrant 😄