Skip to content

Conversation

@ya-woodcutter
Copy link
Contributor

ff & safari (isomorphic-fetch) doesn't support response.body

@coveralls
Copy link

coveralls commented Mar 16, 2017

Coverage Status

Coverage decreased (-1.7%) to 98.333% when pulling 6b68d1f on ya-woodcutter:fix-ff-safari into 4f0ffda on auru:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a03047b on ya-woodcutter:fix-ff-safari into 4f0ffda on auru:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a03047b on ya-woodcutter:fix-ff-safari into 4f0ffda on auru:master.

@coveralls
Copy link

coveralls commented Mar 16, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling a03047b on ya-woodcutter:fix-ff-safari into 4f0ffda on auru:master.

return fetch(url, {...defaults.fetchOptions, ...fetchOptions, ...options})
.then( result => {
const promiseCallback = result.body ? result[method]() : new Promise(resolve => resolve(result.body));
const promiseCallback = result[method]();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ya-woodcutter according to https://developer.mozilla.org/en-US/docs/Web/API/Response
Response implements Body. I think the issue is with isomorphic-fetch not being really isomorphic and using node-fetch underneath.
Did you try using Response.blob() instead?

const promiseCallback =  result[result.body ? method : 'blob']();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue is with isomorphic-fetch not being really isomorphic...

Isomorphic-fetch implements the minimal functionality of fetch.

Did you try using Response.blob() instead?

I didn't try to do it. But I think that the transform data from Blob to the right data type will be problematic. It requires additional conditions.

I'll try to do something in this way.

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 19a4b6f on ya-woodcutter:fix-ff-safari into 4f0ffda on auru:master.

@ya-woodcutter
Copy link
Contributor Author

Now:

  • We are trying to call response[method]()
  • First catch:
    • if !ok then throw new Error(status, statusText)
    • if ok then return body || null (If we are in this catch, then an error in getting data)
  • Then:
    • if !ok then throw new Error(status, statusText, body)
    • if ok then return body
  • Second catch: return error

@TimeRaider
Copy link
Contributor

@ya-woodcutter Sorry I've fucked up your pr

@ya-woodcutter
Copy link
Contributor Author

@TimeRaider ? emm... it's ok :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f6c360a on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f6c360a on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f6c360a on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling d9e87b8 on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ec4e3af on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

1 similar comment
@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ec4e3af on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@ya-woodcutter
Copy link
Contributor Author

UP: @TimeRaider @evilj0e @Blackheart340

})
.then( response => response[method]()
.catch( () => {
if (!response.ok) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the same codestyle everywhere

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c67fc56 on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c67fc56 on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling c67fc56 on ya-woodcutter:fix-ff-safari into 0d946d1 on auru:master.

@TimeRaider TimeRaider merged commit 15c3065 into auru:master Mar 17, 2017
@ya-woodcutter ya-woodcutter deleted the fix-ff-safari branch March 17, 2017 13:53
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

Successfully merging this pull request may close these issues.

5 participants