Skip to content
Snippets Groups Projects

Update error handling for fetch

Merged Coby Sher requested to merge issue/api_client-3407051:error-handling into canary
All threads resolved!

We can add an onError option in the future that will be called instead of throwing the error, but for now it's good to have at least some way to bubble up errors from fetch

  • Refactor the return type for fetch
    • fetch now returns an object with the response if no errors occurred, or an error if one occurred.
    • the error will need to be handled by the method calling fetch. at the moment the methods will simply re-throw the error and log it to the console if debug is on - at the moment we are also logging in fetch regardless of debug. This part I'm least sure about. Should we move debug to the base class and only log in fetch if debug is true. Either way seems ok to me but I would like a second opinion.
Edited by Coby Sher

Merge request reports

Pipeline #67611 passed

Pipeline passed for 076309e0 on issue:error-handling

Approved by

Merged by Brian PerryBrian Perry 1 year ago (Jan 15, 2024 10:15pm UTC)

Merge details

  • Changes merged into canary with a019046a (commits were squashed).
  • Did not delete the source branch.

Pipeline #77730 passed

Pipeline passed for a019046a on canary

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Brian Perry
  • Should we move debug to the base class and only log in fetch if debug is true.

    That makes sense to me if you want to make that change on this MR.

  • Just a couple of small typos in the example. And I do agree that it makes sense to move the debug logging into the base class so we can control that in fetch as well.

  • Coby Sher added 1 commit

    added 1 commit

    Compare with previous version

  • Coby Sher resolved all threads

    resolved all threads

  • Coby Sher added 1 commit

    added 1 commit

    Compare with previous version

  • Brian Perry approved this merge request

    approved this merge request

  • merged

  • Please register or sign in to reply
    Loading