bugsnag / bugsnag-source-maps

CLI and JS library for uploading source maps to BugSnag
MIT License
16 stars 9 forks source link

Browser.updloadMultiple is slower than running many Browser.uploadOne #77

Open agvald opened 2 years ago

agvald commented 2 years ago

Description

We have recently started using esbuild, switching away from sprockets. (Runnig a rails app)
In this process we starting shipping our sourceMaps to bugsnag.

At this point we have about 50 assets which are processed by esbuild and shipped to bugsnag, but we ship them as both esModules and as CommonJS so essentially 80 files we need to upload+their sourcemaps.

With using browser.uploadMultiple this takes about 45-50 secs, but when we do schedule all at once(with one call to browser.uploadOne per file and a Promise.all wrapping it) we end up spending 6-7 secs on upload.

  return Promise.all(
    filePaths.flatMap((filePath) => {
      return [
        browser.uploadOne({...uploadArgs, bundleUrl: `https://${process.env.DOMAIN}/dist/${filePath}`, sourceMap: `${filePath}.map`}),
      ]
    })
  )

Describe the solution you'd like I would like browser.uploadMultiple to to find all the files on disk and schedule all requests at the same time so they will be processed in parallel.

Describe alternatives you've considered I found my work around so im fine atm. But i don't know how good your servers handle it when i suddenly ship 1000 sourcemaps at the same time, I did not implement any limiting, but that is a thing which I belive you could easily make people do if you provide it through browser.uploadMultiple

We will probably hit about 1000 files when we move all our assets from sprockets to esbuild.

I belive this would make people happy, since their deploys will be faster, and if 100 people ship 1000 assets at the same time people might start getting weird errors.

mattdyoung commented 2 years ago

Hi @agvald

Thanks for the suggestion! This seems a reasonable feature request, although we'd probably need to add a new option to control this as there could be issues with the upload speed if too many uploads are attempted concurrently.

agvald commented 2 years ago

Thanks for a response @mattdyoung!

What also occurred to me as a thing people might want, or rather that I want. WDYT about this? I run two concurrent builds, in esm and cjs, they output to the same folder, but I do the uploads concurrently, which means, I need to be able to pass a list of files to process instead of a folder, I can have a go at implementing something if you'd like that? Would you like me to replace the UploadMultiple or just add an extra function/alternative?

Currently i loop over the list of files for each build and call UploadOne, which is actually pretty simple, but it wont be that simple once I should add some limiting. I would go with a default limiting at 100 concurrent uploads.

I would add the code and specs, what about docs, should I touch the docs, how/where?

yousif-bugsnag commented 2 years ago

Hi @agvald,

Thanks for the additional suggestion! While we can see how passing a list of files rather than a directory would be useful in that scenario, it doesn't feel like a particularly common use case, and could make things more confusing for other customers.

If you need to run two concurrent builds that output to the same directory your current workaround using uploadOne seems appropriate.

krazibit commented 2 years ago

Yeah, this is an issue for large codebase, we have currently about 800 files and it take up to 10 minutes. is there a possibility to send zipped and the server unzips on your end?

xljones commented 2 years ago

is there a possibility to send zipped and the server unzips on your end?

At present, it's not possible to do this, but have reported your feedback on this for future consideration.