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

Add headers to return #42

Merged
merged 2 commits into from
Mar 20, 2017
Merged

Add headers to return #42

merged 2 commits into from
Mar 20, 2017

Conversation

jnutter
Copy link
Contributor

@jnutter jnutter commented Mar 16, 2017

Return headers to promise resolve.

Use case: When fetching pagination data, our API returns a next page header.

@jnutter
Copy link
Contributor Author

jnutter commented Mar 16, 2017

Do you think its better to return the whole response instead? I would like the response.ok as well as err.

@ryanashcraft
Copy link
Contributor

@jnutter You'll need to update this PR because of the changes from #23.

Let's call the field headers when supplied to the promise callback. Additionally, can you also go ahead and start including it in the REQUEST_SUCCESS / REQUEST_FAILURE / MUTATE_SUCCESS / MUTATE_FAILURE actions with the field name responseHeaders? See #39 for a reference here.

I want to avoid passing variables that are too specific to superagent. You should be ok (pun intended 😝) with computing ok from status in your app. See the "Response status" section in superagent's docs for how it computes response.ok.

@jnutter
Copy link
Contributor Author

jnutter commented Mar 20, 2017

@ryanashcraft please let me know if there is anything else 😄

@ryanashcraft ryanashcraft merged commit 65a7995 into amplitude:master Mar 20, 2017
@ryanashcraft
Copy link
Contributor

thanks for contributing again @jnutter!

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.

2 participants