smart-on-fhir / sample-apps-stu3

Collection of simple sample apps
Other
47 stars 43 forks source link

Prompt user to retry on transient error #23

Closed djake closed 3 years ago

djake commented 3 years ago

This PR adds support for retries in the event of transient error(s).

I don't have a readily available mechanism to trigger such an error, so this code is untested.

There is no backoff logic since retry is triggered by user interaction.

Line 633 is an unrelated change but similarly a case of retrying. By moving that block of code into a run function we still have the catch after calling downloadFhir, so I thought that was appropriate.

I have confirmed that the existing base case error handling is displayed to the user:

$ node . -F foo   
Authorizing...
400 Bad Request
 processing error:  The "foo" _outputFormat is not supported
vlad-ignatov commented 3 years ago

Unfortunately, this does not seem to work correctly. To manually try this, connect to a bulk data server configured to throw transient errors (like https://bulk-data.smarthealthit.org/?auth_type=jwks&err=transient_error&jwks=eyJrZXlzIjpbeyJrdHkiOiJFQyIsImNydiI6IlAtMzg0IiwieCI6InZHZnVrai1oTzZLQ3d4Y1Bmdl9fUmlWb3FsdGdZNjVjLVVMYVRHX2ZkZDM0ME9lSDUtMjlObkxPazVDUGhZc2oiLCJ5IjoiNEh4TEp2LXdsRVBBNWUzNVVrdlVhVjU4Rlp1Y3NvQmc1WHFDLTBiQThlS2lEclo1eVFXMDNBN0U1ZXB4VmtJRyIsImtleV9vcHMiOlsidmVyaWZ5Il0sImV4dCI6dHJ1ZSwia2lkIjoiNzdhZjkyYzRmN2I5OTlkNzgzMmEzZDEzYjA3ZmIxMTQiLCJhbGciOiJFUzM4NCJ9LHsia3R5IjoiRUMiLCJjcnYiOiJQLTM4NCIsImQiOiJGLTYyNm8xQjZONzZhODFWSGgtOGhqVWduMkNTZlBua2JpRHU2TzJxdnBWVFpzbEZQajVvb2o5U0RVZUJUZEpZIiwieCI6InZHZnVrai1oTzZLQ3d4Y1Bmdl9fUmlWb3FsdGdZNjVjLVVMYVRHX2ZkZDM0ME9lSDUtMjlObkxPazVDUGhZc2oiLCJ5IjoiNEh4TEp2LXdsRVBBNWUzNVVrdlVhVjU4Rlp1Y3NvQmc1WHFDLTBiQThlS2lEclo1eVFXMDNBN0U1ZXB4VmtJRyIsImtleV9vcHMiOlsic2lnbiJdLCJleHQiOnRydWUsImtpZCI6Ijc3YWY5MmM0ZjdiOTk5ZDc4MzJhM2QxM2IwN2ZiMTE0IiwiYWxnIjoiRVMzODQifV19)

  1. The lib.ask(...) call needs to have a return in front of it. Otherwise it does not wait for the user to answer.
  2. Transient errors are somewhat special. They are thrown once but disappear after retry. It does not work that way here because the entire export is restarted, including the kick-off. However, these errors can only appear on the status endpoint and that is the retry-able part of the flow.

I was able to make that work locally but I guess it would be better to let you fix it and merge the PR then. My quick and not well-tested fix was to move the "ask" code from its current location to the downloadFhir function:

function downloadFhir() {
    // ...
    return kickOff()
    .then(res => {
        console.log("Waiting for the server to generate the files...".green);
        STATUS_URL = res.headers["content-location"];
        return waitForFiles();
    })
    .catch(err => {
        if(err.transient) {
            return lib.ask("Operation failed due to a transient error. Would you like to retry? [Y/n]")
            .then(answer => {
                if(answer.toLowerCase() === 'y') {
                    return waitForFiles();
                }
                throw new Error("User canceled")
            })
        }
        throw err;
    })
    .then(files => {
    // ...
vlad-ignatov commented 3 years ago

Thanks Jake!