[PATCH] qmgr/tests: Fix issues with handling of transient errors

aaron aa1ronham at gmail.com
Mon Oct 26 19:00:49 CDT 2020


There's a single failure, which appears to be a spurious network issue
unrelated to this patch

On Sun, Oct 25, 2020 at 11:51:25PM -0400, Aaron Hill wrote:
> When a BITS job is being transferred, it may enter into the state
> BG_JOB_STATE_TRANSIENT_ERROR (for example, if the hostname fails to
> resolve). Currently, entering this state causes qmgr job tests to fail,
> even though it may occur due to temporary network issues out of our
> control.
> 
> If a job enters BG_JOB_STATE_TRANSIENT_ERROR before the timeout has
> elapsed, attempt to resume the job using
> IBackgroundCopyJob_Resume. If the job is still in
> BG_JOB_STATE_TRANSIENT_ERROR, query BITS for detailed error
> information, and print it out.
> 
> Additionally, ensure that we are able to transfer files on Windows 10
> with a metered connection. By default, BITS will not attempt to transfer
> a job on a metered connection, instead failing with
> BG_JOB_STATE_TRANSIENT_ERROR. On newer versions of Windows, we can
> use IBackgroundCopyJob5 to set the transfer policy, forcing the job to
> run even on a metered connection. This allows qmgr job tests to pass on
> the testbot Windows 10 VMs, which have metered connections enabled in
> order to disable Windows Update.
> 
> Setting the job transfer policy requires using APIs from BITS 5.0. Add
> the minimal amount of the interface needed for the test.
> 
> With this patch, the qmgr job tests now pass on all testbot VMs.
> 
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50048
> Signed-off-by: Aaron Hill <aa1ronham at gmail.com>
> ---
>  dlls/qmgr/qmgr_local.idl |   1 +
>  dlls/qmgr/tests/job.c    | 117 +++++++++++++++++++++++++++++++++++++--
>  include/Makefile.in      |   1 +
>  include/bits5_0.idl      |  36 ++++++++++++
>  4 files changed, 151 insertions(+), 4 deletions(-)
>  create mode 100644 include/bits5_0.idl
> 
> diff --git a/dlls/qmgr/qmgr_local.idl b/dlls/qmgr/qmgr_local.idl
> index 110d479ab92..48439fc5dac 100644
> --- a/dlls/qmgr/qmgr_local.idl
> +++ b/dlls/qmgr/qmgr_local.idl
> @@ -25,3 +25,4 @@
>  #include "bits2_0.idl"
>  #include "bits2_5.idl"
>  #include "bits3_0.idl"
> +#include "bits5_0.idl"
> diff --git a/dlls/qmgr/tests/job.c b/dlls/qmgr/tests/job.c
> index 53b724eadbf..68d15acaf0a 100644
> --- a/dlls/qmgr/tests/job.c
> +++ b/dlls/qmgr/tests/job.c
> @@ -27,6 +27,7 @@
>  #include "initguid.h"
>  #include "bits2_0.h"
>  #include "bits2_5.h"
> +#include "bits5_0.h"
>  
>  /* Globals used by many tests */
>  static WCHAR test_remotePathA[MAX_PATH];
> @@ -75,6 +76,8 @@ static void init_paths(void)
>  static BOOL setup(void)
>  {
>      HRESULT hres;
> +    IBackgroundCopyJob5* test_job_5;
> +    BITS_JOB_PROPERTY_VALUE prop_val;
>  
>      test_manager = NULL;
>      test_job = NULL;
> @@ -95,6 +98,25 @@ static BOOL setup(void)
>          return FALSE;
>      }
>  
> +    /* The Wine TestBot Windows 10 VMs disable Windows Update by putting
> +       the network connection in metered mode (see https://wiki.winehq.org/Wine_TestBot_VMs#Windows_configuration)
> +
> +       Unfortunately, this will make BITS jobs fail, since the default transfer policy
> +       on Windows 10 prevents BITs job from running over a metered network
> +
> +       To allow these tests in this file to run on the testbot, we
> +       set the BITS_JOB_PROPERTY_ID_COST_FLAGS property to BITS_COST_STATE_TRANSFER_ALWAYS,
> +       ensuring that BITS will still try to run the job on a metered network */
> +    prop_val.Dword = BITS_COST_STATE_TRANSFER_ALWAYS;
> +    hres = IBackgroundCopyJob_QueryInterface(test_job, &IID_IBackgroundCopyJob5, (void **)&test_job_5);
> +    /* BackgroundCopyJob5 was added in Windows 8, so this may not exist. The metered connection
> +       workaround is only applied on Windows 10, so it's fine if this fails. */
> +    if (SUCCEEDED(hres)) {
> +        hres = IBackgroundCopyJob5_SetProperty(test_job_5, BITS_JOB_PROPERTY_ID_COST_FLAGS, prop_val);
> +        ok(hres == S_OK, "Failed to set the cost flags: %08x\n", hres);
> +        IBackgroundCopyJob5_Release(test_job_5);
> +    }
> +
>      return TRUE;
>  }
>  
> @@ -333,6 +355,43 @@ static void compareFiles(WCHAR *n1, WCHAR *n2)
>      ok(memcmp(b1, b2, s1) == 0, "Files differ in contents\n");
>  }
>  
> +/* Handles a timeout in the BG_JOB_STATE_ERROR or BG_JOB_STATE_TRANSIENT_ERROR state */
> +static void handle_job_err(void)
> +{
> +    HRESULT hres;
> +    IBackgroundCopyError *err;
> +    BG_ERROR_CONTEXT errContext;
> +    HRESULT errCode;
> +    LPWSTR contextDesc;
> +    LPWSTR errDesc;
> +
> +    hres = IBackgroundCopyJob_GetError(test_job, &err);
> +    if (SUCCEEDED(hres)) {
> +        hres = IBackgroundCopyError_GetError(err, &errContext, &errCode);
> +        if (SUCCEEDED(hres)) {
> +            ok(0, "Got context: %d code: %d\n", errContext, errCode);
> +        } else {
> +            ok(0, "Failed to get error info: 0x%08x\n", hres);
> +        }
> +
> +        hres = IBackgroundCopyError_GetErrorContextDescription(err, MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), &contextDesc);
> +        if (SUCCEEDED(hres)) {
> +            ok(0, "Got context desc: %s\n", wine_dbgstr_w(contextDesc));
> +        } else {
> +            ok(0, "Failed to get context desc: 0x%08x\n", hres);
> +        }
> +
> +        hres = IBackgroundCopyError_GetErrorDescription(err, MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), &errDesc);
> +        if (SUCCEEDED(hres)) {
> +            ok(0, "Got error desc: %s\n", wine_dbgstr_w(errDesc));
> +        } else {
> +            ok(0, "Failed to get error desc: 0x%08x\n", hres);
> +        }
> +    } else {
> +        ok(0, "Failed to get error: 0x%08x\n", hres);
> +    }
> +}
> +
>  /* Test a complete transfer for local files */
>  static void test_CompleteLocal(void)
>  {
> @@ -362,13 +421,23 @@ static void test_CompleteLocal(void)
>          hres = IBackgroundCopyJob_GetState(test_job, &state);
>          ok(hres == S_OK, "IBackgroundCopyJob_GetState\n");
>          ok(state == BG_JOB_STATE_QUEUED || state == BG_JOB_STATE_CONNECTING
> -           || state == BG_JOB_STATE_TRANSFERRING || state == BG_JOB_STATE_TRANSFERRED,
> +           || state == BG_JOB_STATE_TRANSFERRING || state == BG_JOB_STATE_TRANSFERRED
> +           || state == BG_JOB_STATE_TRANSIENT_ERROR,
>             "Bad state: %d\n", state);
> +
> +        if (state == BG_JOB_STATE_TRANSIENT_ERROR) {
> +            hres = IBackgroundCopyJob_Resume(test_job);
> +            ok(hres == S_OK, "IBackgroundCopyJob_Resume\n");
> +        }
> +
>          if (state == BG_JOB_STATE_TRANSFERRED)
>              break;
>          Sleep(1000);
>      }
>  
> +    if (state == BG_JOB_STATE_ERROR || state == BG_JOB_STATE_TRANSIENT_ERROR)
> +        handle_job_err();
> +
>      ok(i < timeout_sec, "BITS jobs timed out\n");
>      hres = IBackgroundCopyJob_Complete(test_job);
>      ok(hres == S_OK, "IBackgroundCopyJob_Complete\n");
> @@ -430,13 +499,24 @@ static void test_CompleteLocalURL(void)
>          hres = IBackgroundCopyJob_GetState(test_job, &state);
>          ok(hres == S_OK, "IBackgroundCopyJob_GetState\n");
>          ok(state == BG_JOB_STATE_QUEUED || state == BG_JOB_STATE_CONNECTING
> -           || state == BG_JOB_STATE_TRANSFERRING || state == BG_JOB_STATE_TRANSFERRED,
> +           || state == BG_JOB_STATE_TRANSFERRING || state == BG_JOB_STATE_TRANSFERRED
> +           || state == BG_JOB_STATE_TRANSIENT_ERROR,
>             "Bad state: %d\n", state);
> +
> +        if (state == BG_JOB_STATE_TRANSIENT_ERROR) {
> +            hres = IBackgroundCopyJob_Resume(test_job);
> +            ok(hres == S_OK, "IBackgroundCopyJob_Resume\n");
> +        }
> +
>          if (state == BG_JOB_STATE_TRANSFERRED)
>              break;
>          Sleep(1000);
>      }
>  
> +    if (state == BG_JOB_STATE_ERROR || state == BG_JOB_STATE_TRANSIENT_ERROR)
> +        handle_job_err();
> +
> +
>      ok(i < timeout_sec, "BITS jobs timed out\n");
>      hres = IBackgroundCopyJob_Complete(test_job);
>      ok(hres == S_OK, "IBackgroundCopyJob_Complete\n");
> @@ -572,11 +652,22 @@ static void test_HttpOptions(void)
>          ok(state == BG_JOB_STATE_QUEUED ||
>             state == BG_JOB_STATE_CONNECTING ||
>             state == BG_JOB_STATE_TRANSFERRING ||
> -           state == BG_JOB_STATE_TRANSFERRED, "unexpected state: %u\n", state);
> +           state == BG_JOB_STATE_TRANSFERRED ||
> +           state == BG_JOB_STATE_TRANSIENT_ERROR, "unexpected state: %u\n", state);
> +
> +        if (state == BG_JOB_STATE_TRANSIENT_ERROR) {
> +            hr = IBackgroundCopyJob_Resume(test_job);
> +            ok(hr == S_OK, "IBackgroundCopyJob_Resume\n");
> +        }
>  
>          if (state == BG_JOB_STATE_TRANSFERRED) break;
>          Sleep(1000);
>      }
> +
> +    if (state == BG_JOB_STATE_ERROR || state == BG_JOB_STATE_TRANSIENT_ERROR)
> +        handle_job_err();
> +
> +
>      ok(i < timeout, "BITS job timed out\n");
>      if (i < timeout)
>      {
> @@ -651,10 +742,28 @@ START_TEST(job)
>      };
>      const test_t *test;
>      int i;
> +    HRESULT hres;
>  
>      init_paths();
>  
> -    CoInitialize(NULL);
> +    /* CoInitializeEx and CoInitializeSecurity with RPC_C_IMP_LEVEL_IMPERSONATE
> +     * are required to set the job transfer policy
> +     * See https://docs.microsoft.com/en-us/windows/win32/bits/how-to-block-a-bits-job-from-downloading-over-an-expensive-connection
> +     */
> +    hres = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
> +    if (FAILED(hres)) {
> +        ok(0, "CoInitializeEx faied: %0x\n", hres);
> +        return;
> +    }
> +
> +    hres = CoInitializeSecurity(NULL, -1, NULL, NULL,
> +                           RPC_C_AUTHN_LEVEL_CONNECT,
> +                           RPC_C_IMP_LEVEL_IMPERSONATE,
> +                           NULL, EOAC_NONE, 0);
> +    if (FAILED(hres)) {
> +        ok(0, "CoInitializeSecurity failed: %0x\n", hres);
> +        return;
> +    }
>  
>      if (FAILED(test_create_manager()))
>      {
> diff --git a/include/Makefile.in b/include/Makefile.in
> index 23306689cab..b2c8ba7f987 100644
> --- a/include/Makefile.in
> +++ b/include/Makefile.in
> @@ -50,6 +50,7 @@ SOURCES = \
>  	bits2_0.idl \
>  	bits2_5.idl \
>  	bits3_0.idl \
> +	bits5_0.idl \
>  	bitsmsg.h \
>  	bluetoothapis.h \
>  	bthsdpdef.h \
> diff --git a/include/bits5_0.idl b/include/bits5_0.idl
> new file mode 100644
> index 00000000000..30b4e4bbe37
> --- /dev/null
> +++ b/include/bits5_0.idl
> @@ -0,0 +1,36 @@
> +#ifndef DO_NO_IMPORTS
> +import "bits.idl";
> +import "bits2_0.idl";
> +import "bits3_0.idl";
> +#endif
> +
> +cpp_quote("#define BITS_COST_STATE_TRANSFER_ALWAYS 0x800000ff")
> +
> +[
> +    uuid(e847030c-bbba-4657-af6d-484aa42bf1fe),
> +    odl
> +]
> +interface IBackgroundCopyJob5: IBackgroundCopyJob4
> +{
> +    typedef enum {
> +        BITS_JOB_PROPERTY_ID_COST_FLAGS = 1,
> +        BITS_JOB_PROPERTY_NOTIFICATION_CLSID = 2,
> +        BITS_JOB_PROPERTY_DYNAMIC_CONTENT = 3,
> +        BITS_JOB_PROPERTY_HIGH_PERFORMANCE = 4,
> +        BITS_JOB_PROPERTY_MAX_DOWNLOAD_SIZE = 5,
> +        BITS_JOB_PROPERTY_USE_STORED_CREDENTIALS = 7,
> +        BITS_JOB_PROPERTY_MINIMUM_NOTIFICATION_INTERVAL_MS = 9,
> +        BITS_JOB_PROPERTY_ON_DEMAND_MODE = 10,
> +    } BITS_JOB_PROPERTY_ID;
> +
> +    typedef union _BITS_JOB_PROPERTY_VALUE {
> +        DWORD Dword;
> +        GUID ClsID;
> +        BOOL Enable;
> +        UINT64 Uint64;
> +        BG_AUTH_TARGET Target;
> +    } BITS_JOB_PROPERTY_VALUE;
> +
> +    HRESULT SetProperty(BITS_JOB_PROPERTY_ID id, BITS_JOB_PROPERTY_VALUE value);
> +    HRESULT GetProperty(BITS_JOB_PROPERTY_ID id, [out, ref] BITS_JOB_PROPERTY_VALUE *value);
> +}
> -- 
> 2.29.1
> 



More information about the wine-devel mailing list