Automattic / knox

S3 Lib
MIT License
1.74k stars 285 forks source link

putFile, res.resume()? #259

Open webchaz opened 9 years ago

webchaz commented 9 years ago

I prefer the putFile syntax, however I just am not sure what the docs mean by always do something with res or at least call res.resume(). I'm checking the data within res, but is there a method I should call to end or what exactly does res.resume() do?

Thank you

morenoh149 commented 9 years ago

Indeed @knox/owners Is there an api of sorts or documentation on what the res looks like? What events it emits and under what circumstances?

For example, I'm trying to catch the case where a putFile operation actually failed to store the file. If I naively say if (res) res.resume() my application reports the file saved successfully instead of issuing an error.

mrjackdavis commented 9 years ago

There seems to be no documentation on res.resume(). We're asked to call this function, without any explanation. That means it's not really possible to make an informed decision as to when to call this method.

Can somebody please improve the documentation on this function

morenoh149 commented 9 years ago

after that message I ended up inspecting the response manually. @mrjackdavis it's an object with many fields. I encourage you to take several minutes and try to understand mostly what's in it. I ended up simply checking the res.statusCode field if it's 200 everything went fine. Else throw an error https://github.com/keystonejs/keystone/pull/1052/files Also it might be useful to read http://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html

mrjackdavis commented 9 years ago

That should help people who are having issues with res. But the real problem is the lack of documentation. We should not have to inspect objects to infer the API.

It would be amazing if it was properly documented

morenoh149 commented 9 years ago

@mrjackdavis indeed. There's a wiki here https://github.com/LearnBoost/knox/wiki like to take a shot given the aws docs? The material isn't fresh in my mind like yours ;)

domenic commented 9 years ago

It's just the standard Node.js/io.js http response object. The docs don't belong here; they already exist in io.js.

mrjackdavis commented 9 years ago

@domenic that's good to know. I didn't see that information in the readme though

domenic commented 9 years ago

Yeah, it's kind of assumed that people working in Node.js environments know how the Node.js HTTP response class works. I guess we could link to it but I mean should we also link to the docs for Number in case people get confused what the type of statusCode is?

mrjackdavis commented 9 years ago

I doubt people will need to know what the type of statusCode is. But really, res might not be a HTTP response class, it could be some sort of custom S3 response. How are they to know?

morenoh149 commented 9 years ago

@domenic if 3 persons have had confusion and bothered to actually participate on this issue I believe a link to the docs for the res object are warranted. What's most important to a new knox user is to understand that the response is from AWS and that they should peruse those docs to understand how to interact with AWS. Yes the onus lies on Knox. Please let me know if this addition would be welcome.

MarZab commented 9 years ago

Lets add all these confused people on that list: https://github.com/LearnBoost/knox/issues/198 It would be really nice to have some boilerplate code that handles some of the use cases.

Beanow commented 8 years ago

In other words: http.IncomingMessage which implements stream.Readable and there it has a resume.

https://nodejs.org/api/stream.html#stream_readable_resume

The readable.resume() method causes an explicitly paused Readable stream to resume emitting 'data' events, switching the stream into flowing mode.

The readable.resume() method can be used to fully consume the data from a stream without actually processing any of that data as illustrated in the following example:

getReadableStreamSomehow()
  .resume();
  .on('end', () => {
    console.log('Reached the end, but did not read anything.');
  });