Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork api_client-3407051
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
coby.sher CreditAttribution: coby.sher as a volunteer commentedI might pick this up. I'm working on https://www.drupal.org/project/api_client/issues/3376946 and finding a few places where it would be nice to throw and error, but this breaks a lot of the current tests. So instead of gilding that one I will start on this.
Here are my thoughts on what the API should look like:
We need to be able to return an error from `fetch`. We can change the return type to a tuple like so:
This has some advantages and some disadvantages: any time `fetch` is called, the result would need to be a destructured array, and we will have to check if Error is not null. This is similar to Golang's convention which can feel kind of verbose, but does enforce error checking and handling.
We could also use an object for this but that detail isn't super important to me. The important part is actually returning and not throwing the error from `fetch`, allowing us to throw or handle the error as the end user sees fit.
Thoughts?
Comment #3
pratik_kamble@coby.sher Make sense to me. Considering 5XX will be treated as normal response only.
Comment #4
brianperryWhat you outlined makes sense to me as well.
> The important part is actually returning and not throwing the error from `fetch`, allowing us to throw or handle the error as the end user sees fit.
Do you have any thoughts on how we'd expose that error handling to users? Anything like what we did for Drupal State: https://project.pages.drupalcode.org/drupal_state/en/error-handling/ or something different?
Comment #5
coby.sher CreditAttribution: coby.sher as a volunteer commentedAt the moment I figured we'd just throw the error and let the user handle it via try/catch. Meaning, when we call our fetch method internally, if the error exists we will throw it. So we will throw from getResource or getCollection but not fetch itself.
We could implement an onError hook. Should that be an enhancement or part of this ticket?
Comment #6
brianperry> At the moment I figured we'd just throw the error and let the user handle it via try/catch. Meaning, when we call our fetch method internally, if the error exists we will throw it. So we will throw from getResource or getCollection but not fetch itself.
That makes sense to me.
> We could implement an onError hook. Should that be an enhancement or part of this ticket?
Given what you outlined, feels like an enhancement.
Comment #8
coby.sher CreditAttribution: coby.sher as a volunteer commentedComment #9
brianperryComment #10
coby.sher CreditAttribution: coby.sher as a volunteer commentedComment #11
brianperryMerged to canary and marked as fixed. Thanks!