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

Error thrown #7

Closed
sdcooke opened this issue Jun 17, 2016 · 19 comments
Closed

Error thrown #7

sdcooke opened this issue Jun 17, 2016 · 19 comments

Comments

@sdcooke
Copy link

sdcooke commented Jun 17, 2016

I've noticed a difference between this network layer and the default network layer. If there is a network error the render method of the Relay.Renderer is called with the error. In the default network layer nothing else is done, in this network layer the error is then thrown after the render function has been called so you can't silently handle it.

I can't work out exactly where it's being thrown from - is this something you're aware of?

@nodkz
Copy link
Collaborator

nodkz commented Jun 17, 2016

Please provide more information

  • screenshot or stack-trace
  • request/response screenshot from network tab in browser
  • and of course you config of relay-network-layer

@sdcooke
Copy link
Author

sdcooke commented Jun 20, 2016

Thanks for your reply. Unfortunately we're not using Relay in the browser yet - just React Native (which could be why this is an issue). This is the stack trace it is reporting:

screen shot 2016-06-20 at 10 39 43

This is the setup I'm using to test:

        const middleware = [
            urlMiddleware({
                url: (): string => this.props.graphql_url
            }),
            (next: Function): any => ((req: any): any => {
                Object.assign(req.headers, this.props.headers);
                return next(req);
            })
        ];
        this.relayEnv.injectNetworkLayer(new RelayNetworkLayer(middleware, {disableBatchQuery: true}));

To get the error I'm just turning off my local graphql server to simulate the network being down. The render prop of the Relay.Renderer component receives the error as it should but the difference between relay-network-layer and the default network layer is that the error from relay-network-layer still causes the react native red error screen but the default network layer does not.

I've looked at the promise coming out of both the default network layer and this network layer and they look exactly the same to me.

If this isn't something you've seen before and there's nothing jumping to mind about why it might be happening I can continue to debug - I was hoping it was something you might have an idea about though!

@nodkz
Copy link
Collaborator

nodkz commented Jun 20, 2016

Thanks for your detailed information.
You may look to this fork bmcmahen@1967a3d and comments to this commit.

I do not have react-native env to debug this issue. But you may! I think problem in whatwg-fetch polyfill. Will be great if you try just remove this line. And confirm stays problem or not.

Please let me know about your investigations.

@sdcooke
Copy link
Author

sdcooke commented Jun 20, 2016

Removing that line didn't fix the problem but I have worked out that it only happens when using remote debugging (i.e. via Chrome). If I use the native javascript engine for the app the problem doesn't happen - so at least it's not an issue that will impact production. I'll close this issue but if you want me to try anything else I'm happy to help.

Thanks - and sorry if I wasted any of your time!

@sdcooke sdcooke closed this as completed Jun 20, 2016
@nodkz
Copy link
Collaborator

nodkz commented Jun 20, 2016

Too strange, cause as you say that default network layer (DNL) does not throw error with remote debugging, or does?

If DNL not produce error, but this network layer do it. So I forced to re-open this issue due difference in behavior. It should be fixed, or described in readme. Anyway if I can not fix it, then somebody may do this and the opened issue only help with it.

@sdcooke
Copy link
Author

sdcooke commented Jun 20, 2016

You're correct - the DNL does not throw the error in debug mode so there is definitely a difference. I've searched the issues on the react native project and can't see anything in there.

@sdcooke sdcooke reopened this Jun 20, 2016
@nodkz
Copy link
Collaborator

nodkz commented Jun 21, 2016

@sdcooke please check new version 1.2.0

Watwg-fetch was removed. May be it produces error.

@sdcooke
Copy link
Author

sdcooke commented Jun 22, 2016

I still get the error, I'm afraid.

@roman01la
Copy link

I'm also having this issue with react-relay-network-layer when network request fails. Basically when I get server response with an error, this error propagates through all Promise rejections (fetch -> fetchWrapper -> ...) and eventually it throws.

@nodkz
Copy link
Collaborator

nodkz commented Jul 12, 2016

Gotcha! Bump new version in hour. @roman01la be ready to test it. Write additionally, when publish on npm.

@roman01la
Copy link

Thanks for quick response!

@nodkz
Copy link
Collaborator

nodkz commented Jul 12, 2016

1.3.0 published

It was some tricky problem. I hope it was solved.

In react-native, if some network problems (eg. offline) fetch throw error not via promise, so to catch this error, I just do such trick:

new Promise((resolve, reject) =>
  fetch(...)
    .then(res => resolve(res))
    .catch(err => reject(err))
);

I suppose that arrow function (provided to promise constructor) catch thrown error and call reject implicitly. Something similar realized in fbjs/fetchWithRetries

@roman01la @sdcooke please try new version, and write about results with your apps.

@roman01la
Copy link

Still the same error :(
I'll dig into RN network code on a sample project and try to find out where's the problem.

@nodkz
Copy link
Collaborator

nodkz commented Jul 12, 2016

@roman01la

Agrrrr. Work in the blind. I have not react-native environment.
Try 1.3.1, just bumped. Change context for function (arrow-functions has callee context).

@nodkz
Copy link
Collaborator

nodkz commented Jul 12, 2016

@sdcooke
Copy link
Author

sdcooke commented Jul 13, 2016

I'm still seeing the same problem in 1.3.1 I'm afraid

@nodkz
Copy link
Collaborator

nodkz commented Oct 11, 2016

Bumped new version 1.3.6.
Please check, the error should be gone.
Thanks to @kosmikko for the fix.

@jamesone
Copy link
Contributor

jamesone commented Sep 5, 2018

I'm seeing the graphql errors being thrown with the latest version of this package.

@ersinakinci
Copy link

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants