Skip to content
Snippets Groups Projects

Add an option to return the raw response.

All threads resolved!

Closes #3391668

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Coby Sher
  • Coby Sher
  • For the test cases, you can check that what is returned from get is instanceof Response. You can also check that response.headers and response.status exists.

    For the msw handlers, you can setup handlers that return different status codes on arbitrary urls for any new things you want to setup, but you should be able to use the existing ones too.

  • shruti shende added 3 commits

    added 3 commits

    • cfd02a9f - 1 commit from branch project:canary
    • c5223d18 - Add an option to return the raw response.
    • 2c54c181 - 3393652: Updates to send raw response.

    Compare with previous version

  • shruti shende added 1 commit

    added 1 commit

    • dfe8684f - 3393652: Adds unit test for raw reponse.

    Compare with previous version

  • Pratik Kamble approved this merge request

    approved this merge request

  • Coby Sher resolved all threads

    resolved all threads

  • shruti shende added 4 commits

    added 4 commits

    • 9d08e674 - 1 commit from branch project:canary
    • 305cc1d8 - Add an option to return the raw response.
    • c1b467bf - 3393652: Updates to send raw response.
    • b2098cb9 - 3393652: Adds unit test for raw reponse.

    Compare with previous version

  • Pratik Kamble added 13 commits

    added 13 commits

    • b2098cb9...620f69bc - 9 commits from branch project:canary
    • c98d30c5 - Add an option to return the raw response.
    • 9af9d28a - 3393652: Updates to send raw response.
    • 74829817 - 3393652: Adds unit test for raw reponse.
    • 7d104d06 - 3391668: Adds tsdoc comment for rawResponse parameter.

    Compare with previous version

    • Resolved by Brian Perry

      The code that is here looks good, but I'm not sure that this is the best API for this. While I'm sure there are some cases where a developer may only want the raw response, I feel like the more common case is that they would want the raw response, along with the data that this client typically provides. Since they have the raw response, they can get whatever they need, but they now have to do it themselves and lose out of all of the other features of the client.

      Thinking of our caching use case in the Pantheon starter kits - don't we need both the response headers and the data from the client? Could be missing something, but with this API it seems like we'd need to either get the raw response and do most of the work that the client does ourselves, or make a second uncached response to the API.

      What if rawResponse (might need a new name in this case) instead returned an object that had both - { response, json }. If we did that, one of those would have to be a copy of the original response since the stream can only be read once.

      Thoughts?

      Edited by Brian Perry
  • Brian Perry added 7 commits

    added 7 commits

    Compare with previous version

  • Brian Perry added 1 commit

    added 1 commit

    • ea18fefe - Update rawResponse object to the format { response, json }

    Compare with previous version

  • Brian Perry resolved all threads

    resolved all threads

  • Coby Sher
  • Needs a changeset and one non-blocking comment, otherwise lgtm.

  • Brian Perry added 1 commit

    added 1 commit

    • 366762d6 - Update example based on feedback and add changeset

    Compare with previous version

  • Brian Perry added 2 commits

    added 2 commits

    • b4f9059b - 1 commit from branch project:canary
    • efdf7b8c - Merge branch 'canary' into 3391668-add-raw-response

    Compare with previous version

  • Coby Sher resolved all threads

    resolved all threads

  • Coby Sher approved this merge request

    approved this merge request

  • merged

  • Please register or sign in to reply
    Loading