Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Electron block blob uploads resuld in DisableResponseDecompressionPolicy not available in browser Error #14025

Closed
2 of 6 tasks
MGibson1 opened this issue Feb 28, 2021 · 9 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-author-feedback Workflow: More information is needed from author to address the issue. no-recent-activity There has been no recent activity on this issue. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Storage Storage Service (Queues, Blobs, Files)

Comments

@MGibson1
Copy link

  • Package Name: @azure/storage-blob
  • Package Version: 12.4.1
  • Operating system: Mac
  • nodejs
    • version:
  • browser
    • name/version: electron/11.1.1
  • typescript
    • version: 3.8.3
  • Is the bug related to documentation in

Describe the bug
I have code shared throughout multiple typescript repositories which references storage-blob to upload files to a block blob using a URI received from a Server which generates a limited-life blob create URI. In Node and browser environments this code works correctly, but my Electron environment complains that: Error: DisableResponseDecompressionPolicy is not supported in browser environment

To Reproduce
Steps to reproduce the behavior:

  1. Implement block blob storage in an electron environment.
  2. Attempt upload using blob URI + SAS
  3. Observer failure

Expected behavior
Library should correctly upload a file when in electron.

Additional context
The library code I'm using can be found here: https://github.com/bitwarden/jslib/blob/direct-upload-and-download-of-send-files/src/services/send.service.ts#L276-L278. This repository is referenced in multiple clients with the expectation that in each an ArrayBuffer is just uploaded to the given blob URI.

The fact that my code works in both Node and browser-based clients leads me to believe that however storage-blob is detecting if it's in a Node environment is being tricked by Electron to the wrong answer.

Any help or a method to just directly send a PUT request to my blob URL would be fantastic.

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 28, 2021
@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Mar 1, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 1, 2021
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Mar 1, 2021
@ramya-rao-a
Copy link
Contributor

Thanks for reporting @MGibson1
@maorleger Did you see something similar when you were trying out the Storage SDKs in electron apps?

@maorleger
Copy link
Member

@ramya-rao-a I don't recall this error and I used that API, but I did have nodeIntegration: true in my sample so Node APIs were available in the renderer/browser process.

When I did run into a different issue in Electron @JasonYeMSFT was a good resource and might be able to know what could be happening.

@ljian3377
Copy link
Member

@MGibson1
Have you set nodeIntegration: true? Could you share more about your code structure?

disableResponseDecompressionPolicy is only available in Node.js.

export const isNode =
  typeof process !== "undefined" &&
  !!process.version &&
  !!process.versions &&
  !!process.versions.node;

  if (isNode) {
    // policies only available in Node.js runtime, not in browsers
    factories.push(proxyPolicy(pipelineOptions.proxyOptions));
    factories.push(disableResponseDecompressionPolicy());
  }

" In Node and browser environments this code works correctly,"

Did we miss anything? Adding @jeremymeng for visibility.

@ljian3377 ljian3377 added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Mar 2, 2021
@MGibson1
Copy link
Author

MGibson1 commented Mar 2, 2021

Hi @ljian3377, Thanks for your reply.

We do have nodeIntegration: true set on the window doing the work.
https://github.com/bitwarden/jslib/blob/df59f99ec649c1f4a75456e1e5a3c0f98de0e9c1/src/electron/window.main.ts#L120

I'm sorry I'm not sure what would be relevant about the code structure. Could you be a little more specific? In general the intention is to request a SAS-authorized url to a specific blob and upload a file to that blob. This occurs in a BrowserWindow with nodeIntegrations: true.

One of my coworkers, @Hinton, noticed that in serviceClient there appears to be an option to turn response decompression off. Is that a viable option to fix?

requestPolicyFactories.push(disableResponseDecompressionPolicy());

I don't think you missed anything in the node detection, and it does return true in my BrowserWindow. The disableResponseDecompressionPolicy is definitly being applied to the request, the issue might be why isn't it working? Is there another location that tests if the policy is valid and may have a different definition of Node being present?

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Mar 2, 2021
@jeremymeng
Copy link
Member

The replacement for browser is specified in core-http package.json "browser" field:

"./es/src/policies/disableResponseDecompressionPolicy.js": "./es/src/policies/disableResponseDecompressionPolicy.browser.js",
.

It seems that somehow Electron picked the browser replacement? Though what I read about was that Electron doesn't follow the browser field.

We could probably change the browser version of disableResponseDecompressionPolicy to log warnings instead of throwing. @bterlson @xirzec @chradek thoughts?

@ljian3377
Copy link
Member

@MGibson1
Could you help verify if the browser version file of disableResponseDecompressionPolicy is being used?
https://github.com/Azure/azure-sdk-for-js/blob/78db83fe148b8dad1fe8db4112cf92b45ffb13d3/sdk/core/core-http/src/policies/disableResponseDecompressionPolicy.ts
https://github.com/Azure/azure-sdk-for-js/blob/ab612a816be2b71e8c33280a298b1cd6b475aa10/sdk/core/core-http/src/policies/disableResponseDecompressionPolicy.browser.ts

Or you can share how to recreate the issue with your repo so @EmmaZhu can recreate it.

And this code is in a quite common code path. It should be already hit if there is such an issue.
Adding our Electron expert @JasonYeMSFT to share some insight on the usage.
https://github.com/bitwarden/jslib/blob/df59f99ec649c1f4a75456e1e5a3c0f98de0e9c1/src/electron/window.main.ts#L120

@EmmaZhu EmmaZhu added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jan 26, 2022
@ghost ghost removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 26, 2022
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Feb 2, 2022
@ghost
Copy link

ghost commented Feb 2, 2022

Hi, we're sending this friendly reminder because we haven't heard back from you in a while. We need more information about this issue to help address it. Please be sure to give us your input within the next 7 days. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@JasonYeMSFT
Copy link
Contributor

JasonYeMSFT commented Feb 2, 2022

I don't have much knowledge to share here since we don't use the SDK in a browser process and we don't use that flag "DisableResponseDecompressionPolicy" neither.

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Feb 2, 2022
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Feb 9, 2022
@ghost
Copy link

ghost commented Feb 9, 2022

Hi, we're sending this friendly reminder because we haven't heard back from you in a while. We need more information about this issue to help address it. Please be sure to give us your input within the next 7 days. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-author-feedback Workflow: More information is needed from author to address the issue. no-recent-activity There has been no recent activity on this issue. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

7 participants