Page MenuHomeElementl

allow step failure events to have no serialized error

Authored by dgibson on Aug 2 2021, 1:06 PM.
Referenced Files
Unknown Object (File)
Sun, Feb 5, 3:36 AM
Unknown Object (File)
Wed, Jan 11, 6:33 PM
Unknown Object (File)
Dec 21 2022, 9:20 PM
Unknown Object (File)
Dec 17 2022, 11:03 AM
Unknown Object (File)
Dec 13 2022, 10:43 PM
Unknown Object (File)
Dec 6 2022, 1:41 PM
Unknown Object (File)
Nov 23 2022, 8:26 AM
Unknown Object (File)
Nov 19 2022, 6:04 PM



StepFailureData lets the error passed in be empty, but then when the event log query goes to create the event, it throws if there is no error set.

This diff makes the server more resilient to empty errors, and changes the frontend to handle step failure events with no serialized error data (falling back to rendering the message of the event instead of the full error, which we have pretty good frontend support for already)

However, before we should land this, we should gain understanding of why this serverside graphql error seems to *completely break the dagit server and stop it from serving any more request until the server is restarted*, at least when it is thrown in certain situations.

Test Plan

BK, raise a step failed error using the k8s executor, dagit no longer bricks, logs from that run render reasonably

Diff Detail

R1 dagster
Lint Not Applicable
Tests Not Applicable

Event Timeline


@alangenfeld i looked over and it wasn't immediately clear to me what the consequences were of removing this line - the reason I did was that it looked bad for step failure events that were just a single line (due to having no stack trace to fill up space - you couldn't even see the context, it was just a big "Click to expand").

what condition did we add where we have a step failure with no exception?


I believe the thinking was that the context header based on the source type is hidden information if the message isn't long enough to trigger the dialog window

step failure event with no serialized error info:

The data classes don't yell and allow an empty error to be passed in (and such events are now in user DBs for better or for worse)

make sure we update the mypy type on StepFailureData which is also not set as Optional

alangenfeld added inline comments.

we should have @dish do a follow up pass for cleanup on these to be safe

This revision is now accepted and ready to land.Aug 2 2021, 5:02 PM

Do you have before/after screenshots?

This revision was landed with ongoing or failed builds.Aug 2 2021, 5:39 PM
This revision was automatically updated to reflect the committed changes.