Feature: Add request hooks

[igoroctaviano]

• Allow users to pass request hooks.
• Related issue: exponential backoff and retry after 500 error OHIF/Viewers#2374
• PR in OHIF: IDC-2374: Exponential backoff and retry after 500 error OHIF/Viewers#2408

Example using node-retry to add retry functionality to XHR:

    const client = new api.DICOMwebClient({
      url: server.qidoRoot,
      headers: DICOMWeb.getAuthorizationHeader(server),
      errorInterceptor: errorHandler.getHTTPErrorHandler(),
      requestHooks: [
        (request, metadata) => {
          function faultTolerantRequestSend(...args) {
            const operation = retry.operation({
              retries: 10,
            });
            /**
             * retries: 5,
             * factor: 3,
             * minTimeout: 1 * 1000,
             * maxTimeout: 60 * 1000,
             * randomize: true
             */

            operation.attempt(function(currentAttempt) {
              const originalOnReadyStateChange = request.onreadystatechange;

              request.onreadystatechange = function() {
                originalOnReadyStateChange.call(request);
                if (request.status === 429 || request.status >= 500) {
                  operation.retry(new Error('Attempt failed!'));
                }
              };

              console.debug(`${metadata.url} (attempt: ${currentAttempt})`);
              request.open(metadata.method, metadata.url, true);
              originalRequestSend.call(request, ...args);
            });
          }

          const originalRequestSend = request.send;
          request.send = faultTolerantRequestSend;

          return request;
        },
      ],
    });

[hackermd]

@igoroctaviano this is super cool, but not easy to understand:

const pipe = functions => args => functions.reduce((arg, fn) => fn(arg), args)

If we go for this approach, I would suggest adding

• Documentation (in the docstring and in the source code) and several examples to describe and demonstrate how this feature is intended to be used
• Runtime type checks (instanceOf, typeof, etc.) to avoid cryptic error messages, for example if a caller passes an object that is not callable
• Unit tests
• Type annotations (we can use the ones we already have in SliM and include them in the library)

[pieper]

Yes, and while we need to be idiomatic to the languages, it would be great if the functionality and perhaps even some of the naming were consistent with the discussion here:

ImagingDataCommons/dicomweb-client#50

[hackermd]

I would suggest adding a couple of runtime checks here

[igoroctaviano]

Updated. Let me know if I should add more. Thanks.

[hackermd]

@igoroctaviano could we call the option requestHooks rather than requestInterceptors? I think the term hook would be more intuitive and is used by other libraries for similar purposes. What do you think?

[igoroctaviano]

@igoroctaviano could we call the option requestHooks rather than requestInterceptors? I think the term hook would be more intuitive and is used by other libraries for similar purposes. What do you think?

Sure, I took it from axios library, where they call these request interceptors but I'm fine with hooks, updated.

[igoroctaviano]

@hackermd just pushed an example and updated the existent one to point to dcmjs server.

[pieper]

It needs to be rebased but otherwise is this ready to be merged?

[igoroctaviano]

It needs to be rebased but otherwise is this ready to be merged?

Yes, just waiting review + merge.

[pieper]

Any further comments @hackermd or should we merge?

[hackermd]

Looks great @igoroctaviano! Just a few minor comments/suggestions. Feel free to ignore.

[Punzo]

@pieper can you please merge this? Thanks!

[hackermd]

@pieper can you please merge this? Thanks!

@Punzo @igoroctaviano I merged the PR. Let me know if we should draft a new release.

[Punzo]

@pieper can you please merge this? Thanks!

@Punzo @igoroctaviano I merged the PR. Let me know if we should draft a new release.

yes, please! thanks!

[hackermd]

@Punzo @igoroctaviano I released version 0.8.0 (https://github.com/dcmjs-org/dicomweb-client/releases/tag/v0.8.0) and uploaded the new version of the package to npm (https://www.npmjs.com/package/dicomweb-client/v/0.8.0)