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

If used with ember-cli-deploy-gzip the uploaded files won't work. #26

Open
urbany opened this issue Nov 3, 2016 · 27 comments
Open

If used with ember-cli-deploy-gzip the uploaded files won't work. #26

urbany opened this issue Nov 3, 2016 · 27 comments

Comments

@urbany
Copy link

urbany commented Nov 3, 2016

Hi, I'm using this addon alongside ember-cli-deploy-gzip and since gzip runs first, the files deploy-sentry uploads to sentry are gzipped and won't work properly.

I tried disabling gzip for map files but since the js files are still being gzipped (as I want them to be on my server) this is what I see on my sentry stacktrace:

screen shot 2016-11-03 at 21 58 24

Any idea on how to solve this? Is it possible to check if the file is gzipped before uploading and if so?

  1. Add a header to the file upload post which will make sentry properly handle the gzipped file?
  2. Or unzip the file before uploading?

Thank you very much in advance

@urbany
Copy link
Author

urbany commented Nov 3, 2016

I see this issue has been brought up before in #8 and #9 but keeping the zipped files isn't a possibility for me. Any ideas on how to fix this for good?

@dschmidt
Copy link
Owner

dschmidt commented Nov 4, 2016

I dont have time to look into this right now :-(

@dschmidt
Copy link
Owner

dschmidt commented Nov 4, 2016

Are you using master or the release version btw?

@urbany
Copy link
Author

urbany commented Nov 4, 2016

Release. I can submit a PR, just need some guidance on what is the best solution.

@dschmidt
Copy link
Owner

dschmidt commented Nov 4, 2016

It would be a start to try master, I'm afraid I can't give any guidance atm. I haven't done any Ember or Node.js work lately - you're probably all on your own here.

@urbany
Copy link
Author

urbany commented Nov 4, 2016

Hehe, ok mate no problem I'll give it a try then.

@duizendnegen
Copy link
Collaborator

We should also push this convo to ember-cli-deploy-gzip, there's cases where we want to keep both the processed files (possibly in distFiles: []) and unprocessed files (possibly in rawFiles: []).

@urbany there are (from my point of view) 3 options, sorted in order of preference:

  1. see if Sentry accepts gzipped assets if we're smarter with headers that we sent to them;
  2. see if we can get the raw/unprocessed files from ember-cli-deploy-gzip;
  3. unzip the files separately again, writing a ember-cli-deploy-ungzip. I vaguely recall other plugins have travelled this route, it would not be my preference at all.

@urbany
Copy link
Author

urbany commented Nov 15, 2016

@duizendnegen thank you for replying, I agree with all your points and priorities.

@urbany
Copy link
Author

urbany commented Nov 18, 2016

So I just spent a few hours trying to solve this and looking at @duizendnegen priorities:

  1. could not make it work, tried using the following headers during upload and it doesn't fix the problem: Transfer-Encoding: gzip, Content-Encoding: gzip
  2. ember-cli-deploy-gzip has an option to keeo the original files, but then the gzipped files have a different name so it's not what we need, and doing something there to keep the originals seems overkill.
  3. This is the only option I can see working now, I can take the idea from Add option to unzip gzipped assets since Sentry can't parse gzipped assets #9 and adapt to current master if you guys are interested.

Right now I decided to stop GZIPping my files and let cloudfront do it for me.

@duizendnegen
Copy link
Collaborator

Could you open a ticket on the Sentry side of things and see if they're seeing similar issues? According to getsentry/sentry#934 this should just work out of the box (but obviously it doesn't)
Thanks for spending some cycles on investigating!

@urbany
Copy link
Author

urbany commented Nov 19, 2016

@duizendnegen I opened a ticket yesterday getsentry/sentry#4566 and I'm currently in contact with their support via email. Current status is: it should work but it's not working, so they are going to fix it. I'll keep you posted when I have an update.

@duizendnegen
Copy link
Collaborator

Any update on this, did support get back to you?

@urbany
Copy link
Author

urbany commented Nov 25, 2016

@duizendnegen nothing yet, still waiting.

@kmiyashiro
Copy link
Contributor

Funny since Sentry explicitly says they do not support gzipped artifacts: https://docs.sentry.io/clients/javascript/sourcemaps/#verify-artifacts-are-not-gzipped

If getsentry/sentry#4566 actually gets fixed and you can send gzipped artifacts with a header, this plugin will have to support setting the header for this to work. As far as I can tell, this is not supported yet: https://github.com/dschmidt/ember-cli-deploy-sentry/blob/master/index.js#L171-L174

@urbany
Copy link
Author

urbany commented Dec 14, 2016

@kmiyashiro when they fix it I'll add a PR with a configurable option for the user to indicate if the sourcemaps are gzipped, in which case the header will be sent.

PS: thanks for pinging the sentry issue.

@kmiyashiro
Copy link
Contributor

@urbany would it be possible to submit the PR before Sentry fixes their side? Since it's supposed to be supported, I don't see why we couldn't add support for it now.

@kmiyashiro
Copy link
Contributor

It's happening 🎉 getsentry/sentry#4677

@duizendnegen
Copy link
Collaborator

Excellent!

fyi we don't need to add an extra configurable option to indicate if the sourcemaps are gzipped, we can simply look at whether context.gzippedFiles is present. See https://github.com/ember-cli-deploy/ember-cli-deploy-s3/blob/master/index.js#L32

@dschmidt
Copy link
Owner

dschmidt commented Dec 15, 2016

Sure, I will accept a PR as soon as it has landed on sentry master. Feel free to send it before the sentry PR is merged.

Great work everyone 👍

urbany added a commit to urbany/ember-cli-deploy-sentry that referenced this issue Dec 15, 2016
urbany added a commit to urbany/ember-cli-deploy-sentry that referenced this issue Dec 15, 2016
urbany added a commit to urbany/ember-cli-deploy-sentry that referenced this issue Dec 15, 2016
urbany added a commit to urbany/ember-cli-deploy-sentry that referenced this issue Dec 15, 2016
@kmiyashiro
Copy link
Contributor

Looks like the sentry PR has been merged.

@alexjpong
Copy link

So it looks like we're still waiting for sentry to merge the gzip code? In the mean time, would it be best to rely on something like Cloudfront to gzip instead of ember-cli-deploy-gzip?

@duizendnegen
Copy link
Collaborator

That's right @alexjpong. There are alternative paths for when Sentry decides it doesn't want to support gzipping, but it's clumsier and much more work (convincing -gzip to keep the non-encrypted files, rename files after uploading to Sentry so that also S3 and azure-blob also find things like they expect...) - I'd rather wait a bit.

@jpaas
Copy link

jpaas commented Jun 27, 2017

This issue hasn't been updated in 6 months. Is there any chance we will be able to use ember-cli-deploy-gzip?

@urbany
Copy link
Author

urbany commented Jun 27, 2017

I asked about this yesterday: getsentry/sentry#4566
No response yet.

@kmiyashiro
Copy link
Contributor

I went the route of unzipping and renaming the unzipped files *.gz before uploading them to sentry with an in-repo addon, I couldn't wait for the fix.

@xcambar
Copy link

xcambar commented Jul 3, 2017

@kmiyashiro care to share? This would help at least the 6 contributors of this thread and probably many more!

@kmiyashiro
Copy link
Contributor

kmiyashiro commented Aug 9, 2017

Sure, here is the in-repo addon, be sure to put this right before the upload to sentry and after you upload your gzipped files where they need to go. In this case, I'm running after s3 which uploads the gzipped files.

/* eslint-env node */
const fs = require('fs');
const glob = require('glob');

module.exports = {
  name: 'ember-cli-deploy-unzip-sentry',

  createDeployPlugin: function(options) {
    return {
      name: options.name,

      runBefore: 'sentry',
      runAfter: ['gzip', 's3'],

      upload: function() {
        let jsFiles = glob.sync('./dist/**/*.js');

        jsFiles.forEach((file) => {
          let fileData = fs.readFileSync(file, 'utf-8');
          fs.writeFileSync(`${file}.gz`, fileData);
          console.log(`✔ copied ${file} to ${file}.gz`);
        });
      },
    };
  }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants