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

Printing uncaught error messages #288

Closed
KyleAMathews opened this issue May 14, 2020 · 14 comments · Fixed by #303
Closed

Printing uncaught error messages #288

KyleAMathews opened this issue May 14, 2020 · 14 comments · Fixed by #303

Comments

@KyleAMathews
Copy link

Something we struggle with Ink is that it doesn't print syntax errors e.g. we try to access an undefined variable. Instead Ink just exits with an error exit code.

Is this something we've misconfigured or is it Ink that's swallowing errors?

@KyleAMathews
Copy link
Author

Figured out the immediate problem by using the node inspector and pausing on uncaught exceptions. Would be nice if Ink caught those errors and printed them out.

@KyleAMathews KyleAMathews changed the title Printing error messages Printing uncaught error messages May 14, 2020
@vadimdemedes
Copy link
Owner

Couldn't reproduce it with:

const Counter = () => throw new Error('Test');
The above error occurred in the <Counter> component:
    in Counter
    in InternalApp

React will try to recreate this component tree from scratch using the error boundary you provided, InternalApp.
Warning: InternalApp: Error boundaries should implement getDerivedStateFromError(). In that method, return a state update to display an error message or fallback UI.
(node:47405) UnhandledPromiseRejectionWarning: Error: test
    at Counter (ink/examples/counter/counter.js:12:9)
    at renderWithHooks (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:6036:18)
    at mountIndeterminateComponent (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:8570:13)
    at beginWork$1 (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:9938:16)
    at Object.invokeGuardedCallbackImpl (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:11563:10)
    at invokeGuardedCallback (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:11740:31)
    at beginWork$$1 (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:15778:7)
    at performUnitOfWork (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:14693:12)
    at workLoopSync (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:14669:22)
    at performSyncWorkOnRoot (ink/node_modules/react-reconciler/cjs/react-reconciler.development.js:14265:11)
(node:47405) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:47405) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Could you send a minimal reproducible example for this issue?

@vadimdemedes
Copy link
Owner

I think this acb6ed2 should help.

@hlolli
Copy link

hlolli commented Jun 4, 2020

This is also happening in our CI, we have gatsby build failing with exit code 1. When reproduced locally we have a memory leak trough yoga-layout-prebuilt which ink depends on. It's quite serious and it's blocking us from developing (because the data we are building has increased so it's suddenly a problem). So most likely I'll be patching ink, but I hope this gets analyzed. This seems to be only problematic on linux, and nodejs 12,13 and 14 all behave the same.

heap

Connected to this ticket, when this happens on our CI with gatsby, we get no information, only after weeks of research do we finally understand why this is happening.

@wardpeet
Copy link
Contributor

wardpeet commented Jun 5, 2020

@hlolli Try running Gatsby with the env var CI=1 it should swap ink with journalist. This should unblock you for now.

@hlolli
Copy link

hlolli commented Jun 5, 2020

@wardpeet thanks! I did try --json to enable the json logger, but ink was still being called, was hopeless about it being configureable. So far so good with CI=1, I'll propogate that knowledge to the others in my team on this gatsby project!

ADDED: too early to celebrate, we are still getting out of memory crashes, going to check if it's still gatsby-cli/ink/yoga-layout-prebuilt

@hlolli
Copy link

hlolli commented Jun 5, 2020

bottom line: Despite CI=1, yoga will still fill the heap and crash __ZN8facebook4yoga14YGNodeToStringEPNSt3__212basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEP6YGNode14YGPrintOptionsj()@17479, that would be a gatsby problem of require-ing it despite it not being used, I assume the same happened when I tested --json

@hlolli
Copy link

hlolli commented Jun 8, 2020

Forget all the noise from me. It was a red herring, that node inspector is totally confusing. After removing ink and yoga_layout from node_modules, I was getting same crash, it will assign percentages / mem-usage to any native module when it's still not related. It seems to be a basic scaling issue, and has nothing to do with the prebuilt yoga-layout modul 🙈

@vadimdemedes
Copy link
Owner

Hey @sindresorhus and @KyleAMathews, I'm working on adding a default global error boundary for Ink and trying to decide between versions of UI for displaying errors. I'd appreciate the vote!

Rich:

CleanShot 2020-06-16 at 22 38 20@2x

Less rich (no code excerpt):

CleanShot 2020-06-16 at 22 39 21@2x

Minimal (no prettified stack):

CleanShot 2020-06-16 at 22 42 07@2x

Extra minimal (strips out stacks from react and react-reconciler packages):

CleanShot 2020-06-16 at 22 44 48@2x

@wardpeet
Copy link
Contributor

I really like the Rich example but maybe hide the stack like you do with the "extra minimal" one.

@vadimdemedes
Copy link
Owner

@wardpeet Will go with the rich version! Going to keep the full stack for now to make it more clear what it is.

@vadimdemedes
Copy link
Owner

Fix is in master, going to keep this issue open until Ink 3 is out.

@sindresorhus
Copy link
Collaborator

I like the rich one too.

@vadimdemedes
Copy link
Owner

vadimdemedes commented Jul 27, 2020

Ink 3 is out with the fix for this issue included! Read the full announcement at https://vadimdemedes.com/posts/ink-3 :)

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

Successfully merging a pull request may close this issue.

5 participants