Page MenuHomeElementl

[dagit] error source
ClosedPublic

Authored by alangenfeld on Feb 25 2021, 9:07 PM.

Details

Summary
  • add enum entries for other types of errors
  • omit stack frames for outer error in dagit log view
Test Plan

before

Screen Shot 2021-02-25 at 3.00.01 PM.png (372×2 px, 194 KB)

after

Screen Shot 2021-02-25 at 2.59.48 PM.png (478×2 px, 435 KB)

dialogs

Screen Shot 2021-02-25 at 4.05.13 PM.png (1×1 px, 1 MB)

Screen Shot 2021-02-25 at 4.04.49 PM.png (802×1 px, 644 KB)

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

alangenfeld edited the test plan for this revision. (Show Details)
alangenfeld added inline comments.
js_modules/dagit/src/app/PythonErrorInfo.tsx
28

I should probably do something with the error source in the pop up here as well

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 25 2021, 9:23 PM
Harbormaster failed remote builds in B26460: Diff 32339!
alangenfeld retitled this revision from [dagit] user_code_error source to [dagit] error source.Feb 25 2021, 11:22 PM
js_modules/dagit/src/app/PythonErrorInfo.tsx
61

It doesn't seem that helpful to actually see the error source (e.g. USER_CODE_ERROR) in the message. Seems more useful to follow-up with steps for recourse.

e.g.

Please check your repository definition for X.

What class of problems are framework errors, as opposed to unexpected errors?

Thanks for picking this up!

js_modules/dagit/src/app/PythonErrorInfo.tsx
58

Did we have text here before? Not a strongly held opinion, but I think that the text inside the error itself already communicates this, so better to have less stuff for users to read.

61

"user provided" -> "user-provided"

This revision is now accepted and ready to land.Feb 26 2021, 1:13 AM

requesting review to get opinions on what to do with dialog context line. My current ordered preference is

  1. just drop the enum name
  2. only show on unexpected error to request for issue filing

what about yall?

js_modules/dagit/src/app/PythonErrorInfo.tsx
58

Ya i think we can iterate to something better here. My thoughts for adding this or something like it:

  • communicating to new&advanced users how the framework models errors
  • adding clarity for newer users or those who cant parse stack traces as quickly
  • trying to help any unexpected exceptions get filed to us
61

ya I was thinking thats useful for us if they copy / screenshot and send it to us

but i do agree its not ideal and we can just match on the description text if we keep it

I think just dropping the enum name is fine. I'm curious how users would separate out the steps to resolve framework vs unexpected errors.

I guess because framework errors might be instance configuration errors?

js_modules/dagit/src/app/PythonErrorInfo.tsx
61

I think it's not a blocker, but I have a natural aversion to having anything in the product talk about "users" and calling anything "user-provided" code. If there's a way to be specific about the error boundary (e.g. this solid name or this schedule definition), we could avoid this distinction.

js_modules/dagit/src/app/PythonErrorInfo.tsx
61

Yeah - I think I share the same aversion.

just show context message on unexpected error

This revision is now accepted and ready to land.Mar 8 2021, 5:29 PM
This revision was automatically updated to reflect the committed changes.