core-http and possibly storage-blob@azure/[email protected], @azure/[email protected]Describe the bug
Unable to use @azure/storage-blob package in a context of web worker. Attempting to load it fails with error
Uncaught ReferenceError: document is not defined
at Module../node_modules/@azure/core-http/es/src/util/xml.browser.js (xml.browser.ts:5)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/policies/deserializationPolicy.js (deserializationPolicy.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/serviceClient.js (serviceClient.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/coreHttp.js (coreHttp.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/storage-blob/dist-esm/storage-blob/src/index.browser.js (index.browser.ts:1)
at __webpack_require__ (bootstrap:19)
The cause of the issue is that document is not available inside WebWorkers.
To Reproduce
Steps to reproduce the behavior:
I created a sample repo showcasing the issue https://github.com/Obi-Dann/azure-storage-blob-worker-issue based on Create React App
Expected behavior
I should be able to use @azure/storage-blob package running it inside a web worker context.
Additional context
I have a web worker process compressing images in background and then uploading them to Azure storage blob. I can only think of sending messages with file buffers/links to the main app from the worker in order to get the files uploaded, but that does not seem very maintainable in my case because the worker needs to do more work once the file is uploaded.
A workaround is below here. Import or place this code at the top of your web worker code. The idea is using 3rd party DOM parser library(xmldom), which is compatible with browser APIs, and mocking document and window.
import { DOMParser, DOMImplementation, XMLSerializer } from "xmldom";
self.DOMParser = DOMParser;
self.XMLSerializer = XMLSerializer;
self.document = {
implementation: new DOMImplementation(),
};
self.window = {
navigator,
};
There is a workaround, but I want to use @azure/storage-blob without any strange mocking and 3rd party library.
@salqueng Thanks for the report! Yes we use DOM api for parsing XML into JSON objects and converting JSON object into XML document. Web workers don't have access to DOM. We will investigate further.
Thanks @salqueng!
There's also an issue with https://github.com/Azure/azure-sdk-for-js/blob/99f531906125f2d22a613957571a32ca792d009d/sdk/core/core-http/src/policies/msRestUserAgentPolicy.browser.ts#L21 which should be quite easy to fix by replacing window.navigator with self.navigator
@bterlson, @xirzec, Any thoughts on how we should proceed here?
We already pull in xml2js for the node case, presumably we may be able to leverage this for the browser case as well (in exchange for bundle expansion, of course.)
We should replace window.navigator with self.navigator, agree with @Obi-Dann there.
I worry about adding yet another required dependency in the browser, but a way to plug in xml2js when you need it seems like a good idea, possibly in addition to making missing xml a late error when you use functionality that depends on it. @jeremymeng is this late error a possibility, or is XML used in too many core scenarios for it to be worth it?
Making missing xml a later error probably wouldn't help storage as most if not all of storage feature depends on it.
Is there any update on this issue? I also found it to be problematic not being able to use Azure Blob Storage's SDK from inside the context of a Web Worker.. The fix suggested above does not work for me; I'm using TypeScript and it complains that self.window/self.document are read-only properties. Does anybody know of a workaround?
@RoyCrivolotti I have not tried, but you might want to try casting (self as any) before assigning to window or document? very hacky though.
So, jut as an update for anyone that might encounter this issue while a more permanent fix is found by Azure's team: The workaround suggested above did indeed work for us in the end, although we did have to tell ts to ignore the obvious issues for it to compile. There was another obscure error we hadn't seen (caused by the worker loader and another package conflicting) that was fixed by debugging our Webpack configuration. Because Webpack errors can sometimes be really cryptic it was hard for us to notice it. Thanks! I'll be checking this issue for the more permanent fix.
Good day!
I'm also experiencing this issue and was just wondering if this issue is on schedule for the Feb 5th target?
I am strangely unable to get the suggested workaround mentioned above to work in my application.
Is it possible that the version of @azure/storage-blob (12.4.0) is referencing a different typescript library from my application and that is causing it to still have document as null?
Kind regards
Willem du Plessis
@WillemDuPlessis could you please share what issues you are having when trying the workaround?
Thank you so much for the feedback! It's unfortunate that DOM APIs are not available in web workers.
What we want to do is support Storage Blobs in web workers without adding unnecessary dependencies to every other
package that does not need XML. The way our packages are currently set up our XML parsing logic lives in core-http, a dependency taken on by every package.
To that end, we have a separate effort in place to move XML parsing into a separate
package that can be consumed independently as needed. Then we can provide a polyfill for XML parsing just for libraries
that depend on it. Unfortunately that makes for a longer term effort (I'll be updating the milestone as per our conversation with @ramya-rao-a )
By the way, if you're interested in more information about our corev2 efforts, feel free to check out our core2 doc!
Hi @jeremymeng
Thanks for the prompt reply.
I've implemented my typescript worker with the workaround as follows:
import { DOMParser, DOMImplementation, XMLSerializer } from "xmldom";
self.DOMParser = DOMParser;
self.XMLSerializer = XMLSerializer;
// eslint-disable-next-line
self.document = {
implementation: new DOMImplementation(),
};
// eslint-disable-next-line
self.window = {
navigator,
};
import BlobUploadService from 'services/blobUploadService'; // This uses @azure/storage-blob to upload
import { BlobAuthToken } from '../models/uploader/BlobAuthToken'; // POO holding SAS token values
import { AuthContextValue } from "[REDACTED]"; // API authentication context
export function upload(
authContext: AuthContextValue,
file: File,
authToken: BlobAuthToken,
progress: (
isLengthComputable: boolean,
loadedDataAmount: number,
totalDataAmount: number
) => void,
error: (errorText: string) => void,
abort: () => void,
load: (p: string | { [key: string]: any }) => void
): Promise<void|string> {
return BlobUploadService.performUpload(
authContext,
file,
authToken,
progress,
error,
abort,
load);
}
I use comlink-loader to to wire it up and add it to the module like this:
declare module 'comlink-loader!*' {
import { AuthContextValue } from "[REDACTED]";
import { BlobAuthToken } from '../models/uploader/BlobAuthToken';
import UploadWorker from './worker';
class WebpackWorker extends UploadWorker {
constructor();
upload(
apiAuthToken: any,
file: File,
authToken: BlobAuthToken,
progress: (
isLengthComputable: boolean,
loadedDataAmount: number,
totalDataAmount: number
) => void,
error: (errorText: string) => void,
abort: () => void,
load: (p: string | { [key: string]: any }) => void
) : Promise<void|string>;
}
export = WebpackWorker;
}
I then expose this by exporting it as:
// eslint-disable-next-line
import UploadWorker from "comlink-loader!./worker"; // inline loader
export default UploadWorker;
An finally implement it in a delegator class:
import UploadWorker from '.';
import { proxy } from 'comlink';
import { AuthContextValue } from '[REDACTED]';
import { BlobAuthToken } from 'models/uploader/BlobAuthToken';
export default class UploadDelegator {
constructor() {
this.worker = new UploadWorker();
}
worker: UploadWorker;
delegate(
authContext: AuthContextValue,
file: File,
authToken: BlobAuthToken,
progress: (
isLengthComputable: boolean,
loadedDataAmount: number,
totalDataAmount: number
) => void,
error: (errorText: string) => void,
abort: () => void,
load: (p: string | { [key: string]: any }) => void): Promise<void|string> {
return this.worker.upload(
authContext.getAccessToken(),
file,
authToken,
proxy(progress),
proxy(error),
proxy(abort),
proxy(load)
)
}
}
When running, though, I still get the error:
Uncaught ReferenceError: document is not defined
at Module../node_modules/@azure/core-http/es/src/util/xml.browser.js (xml.browser.ts:7)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/policies/deserializationPolicy.js (deserializationPolicy.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/serviceClient.js (serviceClient.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/core-http/es/src/coreHttp.js (coreHttp.ts:1)
at __webpack_require__ (bootstrap:19)
at Module../node_modules/@azure/storage-blob/dist-esm/storage-blob/src/index.browser.js (index.browser.ts:1)
at __webpack_require__ (bootstrap:19)
Should the workaround be placed in the @azure/core-http/es/src/util/xml.browser.js file instead? That would have some build and release implications that may be tricky, but could still be possible.
@WillemDuPlessis Thanks for sharing the repro code. It feels that our storage module got loaded before the polyfill. I am not familiar with comlink-loader. We will investigate more.
We no longer use window, preferring self instead. That fixes the first issue as of @azure/core-http 1.2.4. A linter rule prevents regressions in that area.
We investigated a few different approaches and it is unfortunate that DOM APIs are not available in web workers. We use them to avoid adding what are relatively heavy dependencies to parse XML. Given the relative infrequency of this issue as well as the unnecessary bundle size bloat that 3rd party dependencies will add we would rather not implement support for XML in web workers out of the box unless we can find a lightweight alternative.
What we will offer is a more helpful message when any of these globals are missing. The early error will help guide developers in the right direction. We will also link to a sample demonstrating how one might polyfill these APIs to support web workers. With those two changes I am closing this issue.
Most helpful comment
Thanks @salqueng!
There's also an issue with https://github.com/Azure/azure-sdk-for-js/blob/99f531906125f2d22a613957571a32ca792d009d/sdk/core/core-http/src/policies/msRestUserAgentPolicy.browser.ts#L21 which should be quite easy to fix by replacing
window.navigatorwithself.navigator