safeStructuredData.readData fails in polyfill.js

[bochaco]

When invoking the safeStructuredData.readData an error is returned:

TypeError: response.buffer is not a function(…)

And it's due to the following line in the structured_data.js code is still incorrect: return response.buffer(); and it should be: return response;

[Yerontour]

I found the same error and also tried your solution, but then I got this as response body: 'body:ReadableStream'

[joshuef]

@bochaco can you do a PR with the fix please

[bochaco]

@joshuef I just sent it: #24

@Yerontour, this is how I read the data:

window.safeStructuredData.readData(AUTH_TOKEN, handleId, '')
        .then((res) => res.json ? res.json() : JSON.parse(new Buffer(res).toString()))
        .then((parsedData) => {...

[joshuef]

Aha, if we're expecting JSON to be returned and not a buffer we can use the standard parseResponse functionality which means there'll be no need for this on the client.

I'm not au fait with the structured data setup at this juncture so I'm not 100% sure what it's supposed to return. But I think if the situation you highlight with your parsing (.then((res) => res.json ? res.json() : JSON.parse(new Buffer(res).toString()))) is something consistent that we get we should perhaps be doing this in safe-js itself (perhaps via an updated parse function)

Thoughts @bochaco ?

[bochaco]

Well, I wasn't aware of those utils functions and I copied that snippet code from somewhere, some of the demo apps I think, but I assume the response could be binary or text/json, so perhaps as you suggested could be nice to have it as part of the safe-js to make it easier and avoid redundancy of that code snippet in the app's code. On the other hand, you know sometimes when the code/app wants to be smarter than it should it gets annoying too, so if someone is storing a text/json but wants to get it as a buffer then... I'm now wondering if having a content-type param (both for writing and reading data) could be worthwhile or just confusing.

[joshuef]

I'm now wondering if having a content-type param (both for writing and reading data) could be worthwhile or just confusing.

Yeh, this is something I've thought about also. I think for the time being though it'll make sense to merge this in so that everything is at least working. And we can consider where to go if safe-js is still useful in its current state once the new SAFE api/ffi setup rolls out.

[loureirorg]

A workaround, considering that all responses are strings:

var promiseTextResponse = function (response) {
  return new Promise(function(resolve, reject) {
    var reader = new window.FileReader();
    reader.readAsText(response);
    reader.onloadend = function() {
      resolve(reader.result);
    }
  });
}

return response.blob().then(promiseTextResponse);