Azure / azure-iot-sdk-node

A Node.js SDK for connecting devices to Microsoft Azure IoT services
https://docs.microsoft.com/en-us/azure/iot-hub/
Other
261 stars 227 forks source link

getBlobSharedAccessSignature calls on device_client.js failing with connection timeout error behind a corporate HTTP proxy. #925

Closed ksin63 closed 3 years ago

ksin63 commented 3 years ago

As an alternate to uploadToBlob method to save SAS token quota from IotHub. I am directly using getBlobSharedAccessSignature and notifyBlobUploadStatus method to control when to acquire token or reuse existing token in case of file upload failure/retry scenario. My workflow is described below

  1. Use IoT SDK to acquire SAS token:
  2. Use Azure Storage SDK to upload file:
  3. Again use IoT SDK to notify IoT Hub about success or failure:

This works perfectly in normal environment with edge having direct internet connectivity, though it fails when the edge-proxy is running behind the corporate firewall with below error. Encountered unrecoverable error: Error: connect ETIMEDOUT 13.79.172.43:443

After a quick glance on the source code it seems the developer in this PR-Add proxy support to blob file upload in Device SDK missed to apply setOptions for FileUploadApi.

A quick fix should go as below

setOptions(options: DeviceClientOptions, done?: Callback<results.TransportConfigured>): Promise<results.TransportConfigured> | void {
    if (!options) throw new ReferenceError('options cannot be falsy.');
    if (this._blobUploadClient) {
      /*Codes_SRS_NODE_DEVICE_CLIENT_99_103: [The `setOptions` method shall set `blobUploadClient` options.]*/
      this._blobUploadClient.setOptions(options);
    }
    /* Implicit call of setOption for fileUploadApi here*/
    if (this._fileUploadApi) {
       /* [The `setOptions` method shall set `fileUploadApi` options.]*/
       this._fileUploadApi.setOptions(options);
    }
    return super.setOptions(options, done);
  }
abossard commented 3 years ago

Would it be possible to backport the fix also to version 1.17.0-lts0720up1?

YoDaMa commented 3 years ago

@abossard we unfortunately only backport security issue patches to LTS releases. Bug fixes like this are the purpose of the main branch.

anthonyvercolano commented 3 years ago

@abossard Is this blocking production?