onProgress not being called

[hashim-sohail]

The function onProgress is not being called.

[robertrossmann]

The function onProgress is being called here:

skipper-better-s3/lib/uploader.js

Lines 106 to 134 in 4c45e4e

	this.trackProgress(request, scope.opts.request.onProgress)
	}
	}
	trackProgress(request, handler) {
	// If there's no handler provided, do not attach any listeners
	if (typeof handler !== 'function') {
	return
	}
	// Generate a per-request unique ID
	const id = uuid.v4()
	request.on('httpUploadProgress', progress => {
	// Prepare Skipper-specific progress structure
	const written = progress.loaded
	// S3 docs say that progress.total may not always be present, so let's have some fallback
	const total = progress.total || request.body.byteCount || null
	const data = {
	id,
	fd: request.body.fd,
	name: request.body.name,
	written,
	total,
	// If total is null, performing bitwise-or will change the result to 0
	// It also rounds down to an integer, which is kinda nice
	// eslint-disable-next-line no-bitwise
	percent: written / total * 100 | 0
	}

Could you show your code that leads you to believe that the function is not being called? Thanks!

[hashim-sohail]

const options =
    {
      adapter: require('skipper-better-s3'),
      key: 'XXX',
      secret: 'XXX',
      bucket: 'XXX',
      s3Params: {Key: file_name},
      region: 'us-west-2',
      saveAs: file_name,
      onProgress: progress => console.log(progress)
    }

I do not see anything on the console.

[robertrossmann]

Thanks. Can you share the code where you actually use that options object? With the upload request etc.

[hashim-sohail]

const options =
    {
      adapter: require('skipper-better-s3'),
      key: 'XXX',
      secret: 'XXX',
      bucket: 'XXX',
      s3Params: {Key: file_name},
      region: 'us-west-2',
      saveAs: file_name,
      onProgress: progress => console.log(progress)
    }


    req.file('file').upload(options,(err, files) => {
      var data = {
        link: 'https://nearpeer-imagees.s3.amazonaws.com/'+file_name,
        success:true
      }
      res.send(data);
    })

This is the whole code

[hashim-sohail]

Does this issue really exists or there is some problem with my code?

[hashim-sohail]

Waiting for your reply?

[robertrossmann]

I think the problem is that you have to pass the progress listener in a sub-object called request:

const options = {
  adapter: require('skipper-better-s3'),
  // ...
  request: {
    onProgress: progress => console.log(progress),
  },
}

Could you try this approach and let me know if this fixes the progress monitoring issue for you? 🙏

This is a bug in skipper-better-s3 if the above code snippet works.

[orenkudev]

@robertrossmann
I tried in my own code adding the following as you suggested:
request: { onProgress: progress => console.log(progress), },
It still not calling the onProgress.
I placed a breakpoint in the uploader.js at line 102. to check the content of scope.opts:

// Attach progress handler, if provided if (typeof scope.opts.request.onProgress === 'function') { this.trackProgress(request, scope.opts.request.onProgress) }
and when looking at the object this is what I have:

The configuration object I have is :
{ adapter: require('skipper-better-s3'), key: process.env.S3_KEY || sails.config.nexcvConfig.S3Settings.KEY, secret: process.env.S3_SECRET || sails.config.nexcvConfig.S3Settings.SECRET, bucket: process.env.S3_BUCKET || sails.config.nexcvConfig.S3Settings.BUCKET, //onProgress: progressUpdate, request: { onProgress: progress => console.log(progress), }, s3params: { ACL: 'public-read' }, }
so for some reason the onProgess doesn't find its way to the scope.opts.request object.

Any thoughts?

========================================
update:
I started looking into the code in order to fix this.
in adapter.js file I added the onProgress configuration:
` const config = {
service: {
accessKeyId: opts.key || defaults.service.accessKeyId,
secretAccessKey: opts.secret || defaults.service.secretAccessKey,
apiVersion: '2006-03-01',
region: opts.region || defaults.service.region,
},

  request: {
    Bucket: opts.bucket || defaults.request.Bucket,
    **onProgress: opts.onProgress || null**
  },

  options: opts.s3options || {},
  // This should only be used when uploading a non-http stream - dirname is usually used
  // by Skipper directly and we receive the full path from it when we receive the stream
  dirname: opts.dirname || null,
}`

so now when calling the adapter from my code I use :
req.file('videoFile').upload({ adapter: require('skipper-better-s3'), key: process.env.S3_KEY || sails.config.nexcvConfig.S3Settings.KEY, secret: process.env.S3_SECRET || sails.config.nexcvConfig.S3Settings.SECRET, bucket: process.env.S3_BUCKET || sails.config.nexcvConfig.S3Settings.BUCKET, //onProgress: this._progressUpdate, onProgress: progress => sails.log.info('loading...'), s3params: { ACL: 'public-read' },

So now it seems to be correct but I still can't see any printouts. I tried debugging it by attaching chrome debugger to the sails app, and I can see that 'trackProgress()' function is called inside uploader.js but it seems like instead of calling my callback function some other empty function is called.
I'm not a javascript expert, this how the function looks inside the debugger :
`
(function() {

})
`
it in a file called VM2040 - I guess it is related to the javascript internal engine

Note, I did try adding console.log call inside 'trackProgress()' and it is working. but it is not calling my provided onProgress.

Any Ideas on how I can move forward with this? I feel I'm very close to resolving this

==================================
One more update after further digging into it. It seems like the merge is changing the function to an anonymous function instead the one provided in the config.
I could really understand why: I am referring to the code in adapter.js line244:
`
receive(opts) {
// Normalise...
opts = opts || {}

opts = merge(Adapter.parseOptions(opts, local(this).globals))
  // Allow the special onProgress event listener to be passed to the upload request
  .and({ request: { onProgress: opts.onProgress } })
  .recursively.into({})

return new Action(opts).upload()

}`

[orenkudev]

After digging into the code, I think I found the source of the bug.
The problem lies in one of the libraries: semantic-merge.

The merge fails when an object value is function. The mergeObject function treats the function as an Object and calls the 'doMerge' function recrusively.
My fix is in line 172:
instead of :
if (opts.recurse && item instanceof Object) {

added a check if the value is not function:
if (opts.recurse && item instanceof Object && !(item instanceof Function)) {
I'll create a pull request. The first time I try pull request so I hope I know what I'm doing