Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

ApolloLink emits both value and error in catch #542

Open
3 of 4 tasks
bmsantos opened this issue Mar 10, 2018 · 1 comment
Open
3 of 4 tasks

ApolloLink emits both value and error in catch #542

bmsantos opened this issue Mar 10, 2018 · 1 comment
Labels
blocking Prevents production or dev due to perf, bug, build error, etc.. good first issue Issues that are suitable for first-time contributors. has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository

Comments

@bmsantos
Copy link

bmsantos commented Mar 10, 2018

Expected Behavior

Observable.catch should emit either recovered value or error.

Actual Behavior

The error is emitted and bubbled up the Rx Observable chain just fine. When caught by the subscriber error action, the observable will be closed and the value emition is never handled causing the app to crash.

A simple reproduction

Place any query and ensure a response with HTTP status code set to anything above 300.
Make sure the errors is set and not null.

See demo project here

Issue Labels

  • has-reproduction
  • feature
  • blocking
  • good first issue

Some details:

I believe the issue is in:

if (err.result && err.result.errors && err.result.data) {

        ...
        .catch(err => {
          if (err.name === 'AbortError') return;
          if (err.result && err.result.errors && err.result.data) {
            observer.next(err.result);
          } 
          observer.error(err);
        });

The above observer.next() ends-up in Promise.reject() in ApolloClient QueryManager.fetchRequest(). The rejection of the Promise will cause the involving Observable stream to be completed.

The above code when modified to the sample below fixes the experienced issues:

        ...
        .catch(err => {
          if (err.name === 'AbortError') return;
          if (err.result && err.result.errors && err.result.data) { // Recover from error and emit value
            observer.next(err.result);
          }  else { // Or just emit error and close stream
            observer.error(err);
          }
        });

Edit by @evans: Attempt to get Apollo Bot to run

@ghost ghost added blocking Prevents production or dev due to perf, bug, build error, etc.. good first issue Issues that are suitable for first-time contributors. has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository labels Mar 12, 2018
pdeszynski added a commit to pdeszynski/apollo-upload-client that referenced this issue Apr 6, 2018
As in the issue apollographql/apollo-link#542 when raising an error and returning a value with observable it will end in an unhandled Promise rejection thats impossible to catch. This change will fix that problem but at the same time currently it looses the possibility to read the recovered value.
melloflavio pushed a commit to indigotech/apollo-link that referenced this issue Apr 17, 2018
melloflavio pushed a commit to indigotech/apollo-link that referenced this issue Apr 18, 2018
@melloflavio
Copy link

melloflavio commented Apr 27, 2018

I've submitted a PR#606 for this fix. What else is needed to incorporate this to develop?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking Prevents production or dev due to perf, bug, build error, etc.. good first issue Issues that are suitable for first-time contributors. has-reproduction ❤ Has a reproduction in a codesandbox or single minimal repository
Projects
None yet
Development

No branches or pull requests

2 participants