subject:"Review Request 63436\: Enabling ErrorBoundary in Scheduler UI"

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-11-09 Thread Reza Motamedi

I ran `./build-support/jenkins/build.sh` and everything looked fine on my
Mac. Can you tell me what the error message was?

Regards,
~RM

On Tue, Nov 7, 2017 at 4:33 PM, David McLaughlin 
wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
>
> On November 8th, 2017, 12:22 a.m. UTC, *Aurora ReviewBot* wrote:
>
> Master (ad86177) is green with this patch.
>   ./build-support/jenkins/build.sh
>
> I will refresh this build result if you post a review containing "@ReviewBot 
> retry"
>
> This fails for me consistently using:
>
> ./build-support/jenkins/build.sh
>
> Is anyone else able to reproduce my failure?
>
>
> - David
>
> On October 31st, 2017, 8:15 p.m. UTC, Reza Motamedi wrote:
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> By Reza Motamedi.
>
> *Updated Oct. 31, 2017, 8:15 p.m.*
> *Repository: * aurora
> Description
>
> Enabling ErrorBoundary in Scheduler UI
>
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
>
> from React docs:
>
> As of React 16, errors that were not caught by any error boundary will result 
> in unmounting of the whole React component tree.
>
> Testing
>
>
>- Unit tests added.
>
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
>
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
>
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
>
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
>
>
>- For more testing, I added the following component (that raises an
>error) and installed it under different components. I attached the
>screen-shots of how this will render. Clicking the link initializes the
>process of Jira ticket creation.
>
> ComponentWithError
>
> import React from 'react'
>
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
>
> Diffs
>
>- ui/.eslintrc (5cdc4e67030a79c3f81c06f585cc9ff5ce959e52)
>- ui/src/main/js/components/ErrorBoundary.js (PRE-CREATION)
>- ui/src/main/js/components/__tests__/ErrorBoundary-test.js
>(PRE-CREATION)
>- ui/src/main/js/index.js (9f94d4bd6f649d74bdd9c3aa99783e01cae25d93)
>
> View Diff 
> File Attachments
> - Apache Jira ticket template filled with information on the bug
> 
> - Catching exceptions on the router
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-11-07 Thread Reza Motamedi

Testing now.

Regards,
~ Reza

On Tue, Nov 7, 2017 at 4:33 PM, David McLaughlin 
wrote:

>
>
> > On Nov. 8, 2017, 12:22 a.m., Aurora ReviewBot wrote:
> > > Master (ad86177) is green with this patch.
> > >   ./build-support/jenkins/build.sh
> > >
> > > I will refresh this build result if you post a review containing
> "@ReviewBot retry"
>
> This fails for me consistently using:
>
> ./build-support/jenkins/build.sh
>
> Is anyone else able to reproduce my failure?
>
>
> - David
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/#review190402
> ---
>
>
> On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/63436/
> > ---
> >
> > (Updated Oct. 31, 2017, 8:15 p.m.)
> >
> >
> > Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> >
> >
> > Repository: aurora
> >
> >
> > Description
> > ---
> >
> > # Enabling ErrorBoundary in Scheduler UI
> > React 16 introduces a new concept of an “error boundary” that allows us
> to limit the impact of an error and not unmount the whole component tree. I
> am open to keeping or removing the stack trace.
> >
> > from React docs:
> > > As of React 16, errors that were not caught by any error boundary will
> result in unmounting of the whole React component tree.
> >
> >
> > Diffs
> > -
> >
> >   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52
> >   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION
> >   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION
> >   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93
> >
> >
> > Diff: https://reviews.apache.org/r/63436/diff/2/
> >
> >
> > Testing
> > ---
> >
> > - Unit tests added.
> >
> > ```
> > # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ?
> [13:06:52]
> > ? ./gradlew ui:test
> >
> > BUILD SUCCESSFUL in 0s
> > 2 actionable tasks: 2 up-to-date
> >
> > # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ?
> [13:07:46]
> > ? ./gradlew ui:lint
> >
> > BUILD SUCCESSFUL in 5s
> > 4 actionable tasks: 4 up-to-date
> > ```
> >
> >
> > - For more testing, I added the following component (that raises an
> error) and installed it under different components. I attached the
> screen-shots of how this will render. Clicking the link initializes the
> process of Jira ticket creation.
> >
> > ## ComponentWithError
> >
> > ```
> > import React from 'react'
> >
> > export default class ComponentWithError extends React.Component {
> >   render() {
> > throw new Error('Crashed!');
> > return ;
> >   }
> > }
> > ```
> >
> >
> > File Attachments
> > 
> >
> > Apache Jira ticket template filled with information on the bug
> >   https://reviews.apache.org/media/uploaded/files/2017/10/
> 31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_
> 2017-10-31_at_11.55.39_AM.png
> > Catching exceptions on the router
> >   https://reviews.apache.org/media/uploaded/files/2017/10/
> 31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_
> 2017-10-31_at_11.44.56_AM.png
> >
> >
> > Thanks,
> >
> > Reza Motamedi
> >
> >
>
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-11-07 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review190402
---


Ship it!




Master (ad86177) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 31, 2017, 1:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 1:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-11-07 Thread David McLaughlin



> On Nov. 8, 2017, 12:02 a.m., David McLaughlin wrote:
> > @ReviewBot retry

Ran into this when trying to commit to master:

PASS src/main/js/components/__tests__/StateMachine-test.js
FAIL src/main/js/components/__tests__/ErrorBoundary-test.js
  ? ErrorBoundary › Should show the error message when catches an exception

expect(received).toBe(expected)

Expected value to be (using ===):
  true
Received:
  false
  
  at Object. 
(src/main/js/components/__tests__/ErrorBoundary-test.js:20:93)
  at process._tickCallback (internal/process/next_tick.js:103:7)

Summary of all failing tests
FAIL src/main/js/components/__tests__/ErrorBoundary-test.js
  ? ErrorBoundary › Should show the error message when catches an exception

expect(received).toBe(expected)

Expected value to be (using ===):
  true
Received:
  false
  
  at Object. 
(src/main/js/components/__tests__/ErrorBoundary-test.js:20:93)
  at process._tickCallback (internal/process/next_tick.js:103:7)


Test Suites: 1 failed, 31 passed, 32 total


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review190392
---


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-11-07 Thread David McLaughlin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review190392
---



@ReviewBot retry

- David McLaughlin


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread David McLaughlin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189781
---


Ship it!




Ship It!

- David McLaughlin


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread Kai Huang


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189779
---


Ship it!





ui/src/main/js/components/__tests__/ErrorBoundary-test.js
Lines 35 (patched)


Pull the common prefix into a constant?


- Kai Huang


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189757
---


Ship it!




Master (d106b4e) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread Joshua Cohen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189756
---


Ship it!




Ship It!

- Joshua Cohen


On Oct. 31, 2017, 8:15 p.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 8:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/2/
> 
> 
> Testing
> ---
> 
> - Unit tests added.
> 
> ```
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:06:52]
> ? ./gradlew ui:test
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? 
> [13:07:46]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 5s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under different components. I attached the screen-shots of 
> how this will render. Clicking the link initializes the process of Jira 
> ticket creation.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> Apache Jira ticket template filled with information on the bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
> Catching exceptions on the router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-31 Thread Reza Motamedi


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/
---

(Updated Oct. 31, 2017, 8:15 p.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.


Repository: aurora


Description
---

# Enabling ErrorBoundary in Scheduler UI
React 16 introduces a new concept of an “error boundary” that allows us to 
limit the impact of an error and not unmount the whole component tree. I am 
open to keeping or removing the stack trace.

from React docs:
> As of React 16, errors that were not caught by any error boundary will result 
> in unmounting of the whole React component tree.


Diffs (updated)
-

  ui/.eslintrc 5cdc4e67030a79c3f81c06f585cc9ff5ce959e52 
  ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
  ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
  ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 


Diff: https://reviews.apache.org/r/63436/diff/2/

Changes: https://reviews.apache.org/r/63436/diff/1-2/


Testing (updated)
---

- Unit tests added.

```
# rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? [13:06:52]
? ./gradlew ui:test

BUILD SUCCESSFUL in 0s
2 actionable tasks: 2 up-to-date

# rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry-test ? [13:07:46]
? ./gradlew ui:lint

BUILD SUCCESSFUL in 5s
4 actionable tasks: 4 up-to-date
```


- For more testing, I added the following component (that raises an error) and 
installed it under different components. I attached the screen-shots of how 
this will render. Clicking the link initializes the process of Jira ticket 
creation.

## ComponentWithError

```
import React from 'react'

export default class ComponentWithError extends React.Component {
  render() {
throw new Error('Crashed!');
return ;
  }
}
```


File Attachments (updated)


Apache Jira ticket template filled with information on the bug
  
https://reviews.apache.org/media/uploaded/files/2017/10/31/0b50175f-0dd5-44fe-8bc4-c69ca1723729__Screen_Shot_2017-10-31_at_11.55.39_AM.png
Catching exceptions on the router
  
https://reviews.apache.org/media/uploaded/files/2017/10/31/915182ef-4229-4b22-b7c6-20a5f24f8e1a__Screen_Shot_2017-10-31_at_11.44.56_AM.png


Thanks,

Reza Motamedi

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread Joshua Cohen



> On Oct. 31, 2017, 2:08 a.m., David McLaughlin wrote:
> > ui/src/main/js/components/ErrorBoundary.js
> > Lines 28 (patched)
> > 
> >
> > Link to the Apache JIRA? And ask them to post the stacktrace below?

Could even create a link that will pre-populate the ticket: 
https://confluence.atlassian.com/jirakb/creating-issues-via-direct-html-links-159474.html.


- Joshua


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189682
---


On Oct. 31, 2017, 1:58 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 1:58 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/sass/app.scss 8a10e1c5a501828c39ca7b9cf6b518a2d7a99d28 
>   ui/src/main/sass/components/_error-boundary.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/1/
> 
> 
> Testing
> ---
> 
> - Unit tests added. This however causes an error message to be logged on 
> stdout.
> 
> ```
> ? ./gradlew ui:test
> ...
> ASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/components/__tests__/ErrorBoundary-test.js
>   ? Console
> 
> console.error node_modules/react-dom/cjs/react-dom.development.js:8305
>   The above error occurred in the  component:
>   in ComponentWithError
>   in ErrorBoundary (created by WrapperComponent)
>   in WrapperComponent
> 
>   React will try to recreate this component tree from scratch using the 
> error boundary you provided, ErrorBoundary.
> ...
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry ? [18:52:22]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 6s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under `UpdateInstanceEvents`. I then caught the error at 
> various depths in the doom tree. I attached the screen-shots of how this will 
> render.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> handling boundary on the main router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/d95e9f85-3dad-4332-ac33-3cb9f8a4455c__Screen_Shot_2017-10-30_at_6.30.22_PM.png
> handling boundary on a component deep in the doom tree.
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/64d47905-b058-4ef5-9d19-fa3311a47f9d__Screen_Shot_2017-10-30_at_6.36.18_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread Kai Huang


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189684
---



+1 to fix the error message in test. Wonder if we could try this? 
https://github.com/facebook/react/issues/11098

- Kai Huang


On Oct. 31, 2017, 1:58 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 1:58 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/sass/app.scss 8a10e1c5a501828c39ca7b9cf6b518a2d7a99d28 
>   ui/src/main/sass/components/_error-boundary.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/1/
> 
> 
> Testing
> ---
> 
> - Unit tests added. This however causes an error message to be logged on 
> stdout.
> 
> ```
> ? ./gradlew ui:test
> ...
> ASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/components/__tests__/ErrorBoundary-test.js
>   ? Console
> 
> console.error node_modules/react-dom/cjs/react-dom.development.js:8305
>   The above error occurred in the  component:
>   in ComponentWithError
>   in ErrorBoundary (created by WrapperComponent)
>   in WrapperComponent
> 
>   React will try to recreate this component tree from scratch using the 
> error boundary you provided, ErrorBoundary.
> ...
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry ? [18:52:22]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 6s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under `UpdateInstanceEvents`. I then caught the error at 
> various depths in the doom tree. I attached the screen-shots of how this will 
> render.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> handling boundary on the main router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/d95e9f85-3dad-4332-ac33-3cb9f8a4455c__Screen_Shot_2017-10-30_at_6.30.22_PM.png
> handling boundary on a component deep in the doom tree.
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/64d47905-b058-4ef5-9d19-fa3311a47f9d__Screen_Shot_2017-10-30_at_6.36.18_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread Aurora ReviewBot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189683
---


Ship it!




Master (87eb891) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 31, 2017, 1:58 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 1:58 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/sass/app.scss 8a10e1c5a501828c39ca7b9cf6b518a2d7a99d28 
>   ui/src/main/sass/components/_error-boundary.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/1/
> 
> 
> Testing
> ---
> 
> - Unit tests added. This however causes an error message to be logged on 
> stdout.
> 
> ```
> ? ./gradlew ui:test
> ...
> ASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/components/__tests__/ErrorBoundary-test.js
>   ? Console
> 
> console.error node_modules/react-dom/cjs/react-dom.development.js:8305
>   The above error occurred in the  component:
>   in ComponentWithError
>   in ErrorBoundary (created by WrapperComponent)
>   in WrapperComponent
> 
>   React will try to recreate this component tree from scratch using the 
> error boundary you provided, ErrorBoundary.
> ...
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry ? [18:52:22]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 6s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under `UpdateInstanceEvents`. I then caught the error at 
> various depths in the doom tree. I attached the screen-shots of how this will 
> render.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> handling boundary on the main router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/d95e9f85-3dad-4332-ac33-3cb9f8a4455c__Screen_Shot_2017-10-30_at_6.30.22_PM.png
> handling boundary on a component deep in the doom tree.
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/64d47905-b058-4ef5-9d19-fa3311a47f9d__Screen_Shot_2017-10-30_at_6.36.18_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread David McLaughlin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/#review189682
---



This looks good. Just to confirm - the behavior we'd see after this ships is 
the screenshot on the left, with the router? Even if a deep component has an 
error?

I'm not a huge fan of the error being logged in the test output. Is there any 
way to avoid that?


ui/src/main/js/components/ErrorBoundary.js
Lines 28 (patched)


Link to the Apache JIRA? And ask them to post the stacktrace below?


- David McLaughlin


On Oct. 31, 2017, 1:58 a.m., Reza Motamedi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63436/
> ---
> 
> (Updated Oct. 31, 2017, 1:58 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> # Enabling ErrorBoundary in Scheduler UI
> React 16 introduces a new concept of an “error boundary” that allows us to 
> limit the impact of an error and not unmount the whole component tree. I am 
> open to keeping or removing the stack trace.
> 
> from React docs:
> > As of React 16, errors that were not caught by any error boundary will 
> > result in unmounting of the whole React component tree.
> 
> 
> Diffs
> -
> 
>   ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/sass/app.scss 8a10e1c5a501828c39ca7b9cf6b518a2d7a99d28 
>   ui/src/main/sass/components/_error-boundary.scss PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63436/diff/1/
> 
> 
> Testing
> ---
> 
> - Unit tests added. This however causes an error message to be logged on 
> stdout.
> 
> ```
> ? ./gradlew ui:test
> ...
> ASS src/main/js/pages/__tests__/Job-test.js
> PASS src/main/js/components/__tests__/InstanceViz-test.js
> PASS src/main/js/pages/__tests__/Update-test.js
> PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
> PASS src/main/js/components/__tests__/Pagination-test.js
> PASS src/main/js/components/__tests__/ErrorBoundary-test.js
>   ? Console
> 
> console.error node_modules/react-dom/cjs/react-dom.development.js:8305
>   The above error occurred in the  component:
>   in ComponentWithError
>   in ErrorBoundary (created by WrapperComponent)
>   in WrapperComponent
> 
>   React will try to recreate this component tree from scratch using the 
> error boundary you provided, ErrorBoundary.
> ...
> 
> BUILD SUCCESSFUL in 0s
> 2 actionable tasks: 2 up-to-date
> 
> # rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry ? [18:52:22]
> ? ./gradlew ui:lint
> 
> BUILD SUCCESSFUL in 6s
> 4 actionable tasks: 4 up-to-date
> ```
> 
> 
> - For more testing, I added the following component (that raises an error) 
> and installed it under `UpdateInstanceEvents`. I then caught the error at 
> various depths in the doom tree. I attached the screen-shots of how this will 
> render.
> 
> ## ComponentWithError
> 
> ```
> import React from 'react'
> 
> export default class ComponentWithError extends React.Component {
>   render() {
> throw new Error('Crashed!');
> return ;
>   }
> }
> ```
> 
> 
> File Attachments
> 
> 
> handling boundary on the main router
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/d95e9f85-3dad-4332-ac33-3cb9f8a4455c__Screen_Shot_2017-10-30_at_6.30.22_PM.png
> handling boundary on a component deep in the doom tree.
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/31/64d47905-b058-4ef5-9d19-fa3311a47f9d__Screen_Shot_2017-10-30_at_6.36.18_PM.png
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Review Request 63436: Enabling ErrorBoundary in Scheduler UI

2017-10-30 Thread Reza Motamedi


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63436/
---

Review request for Aurora, David McLaughlin, Joshua Cohen, and Kai Huang.


Repository: aurora


Description
---

# Enabling ErrorBoundary in Scheduler UI
React 16 introduces a new concept of an “error boundary” that allows us to 
limit the impact of an error and not unmount the whole component tree. I am 
open to keeping or removing the stack trace.

from React docs:
> As of React 16, errors that were not caught by any error boundary will result 
> in unmounting of the whole React component tree.


Diffs
-

  ui/src/main/js/components/ErrorBoundary.js PRE-CREATION 
  ui/src/main/js/components/__tests__/ErrorBoundary-test.js PRE-CREATION 
  ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
  ui/src/main/sass/app.scss 8a10e1c5a501828c39ca7b9cf6b518a2d7a99d28 
  ui/src/main/sass/components/_error-boundary.scss PRE-CREATION 


Diff: https://reviews.apache.org/r/63436/diff/1/


Testing
---

- Unit tests added. This however causes an error message to be logged on stdout.

```
? ./gradlew ui:test
...
ASS src/main/js/pages/__tests__/Job-test.js
PASS src/main/js/components/__tests__/InstanceViz-test.js
PASS src/main/js/pages/__tests__/Update-test.js
PASS src/main/js/components/__tests__/InstanceHistoryItem-test.js
PASS src/main/js/components/__tests__/Pagination-test.js
PASS src/main/js/components/__tests__/ErrorBoundary-test.js
  ? Console

console.error node_modules/react-dom/cjs/react-dom.development.js:8305
  The above error occurred in the  component:
  in ComponentWithError
  in ErrorBoundary (created by WrapperComponent)
  in WrapperComponent

  React will try to recreate this component tree from scratch using the 
error boundary you provided, ErrorBoundary.
...

BUILD SUCCESSFUL in 0s
2 actionable tasks: 2 up-to-date

# rmotamedi@tw-mbp-rmotamedi:~/oss/aurora on git:error-boundry ? [18:52:22]
? ./gradlew ui:lint

BUILD SUCCESSFUL in 6s
4 actionable tasks: 4 up-to-date
```


- For more testing, I added the following component (that raises an error) and 
installed it under `UpdateInstanceEvents`. I then caught the error at various 
depths in the doom tree. I attached the screen-shots of how this will render.

## ComponentWithError

```
import React from 'react'

export default class ComponentWithError extends React.Component {
  render() {
throw new Error('Crashed!');
return ;
  }
}
```


File Attachments


handling boundary on the main router
  
https://reviews.apache.org/media/uploaded/files/2017/10/31/d95e9f85-3dad-4332-ac33-3cb9f8a4455c__Screen_Shot_2017-10-30_at_6.30.22_PM.png
handling boundary on a component deep in the doom tree.
  
https://reviews.apache.org/media/uploaded/files/2017/10/31/64d47905-b058-4ef5-9d19-fa3311a47f9d__Screen_Shot_2017-10-30_at_6.36.18_PM.png


Thanks,

Reza Motamedi

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Re: Review Request 63436: Enabling ErrorBoundary in Scheduler UI

Review Request 63436: Enabling ErrorBoundary in Scheduler UI

15 matches

Site Navigation

Mail list logo

Footer information