Skip to content

Commit

Permalink
[dagit] add full serialized error to graphql errors (#12228)
Browse files Browse the repository at this point in the history
Use the extensions field on the error contents to include the full
`SerializableErrorInfo` object for the original exception. This allows
us to see the stack trace / cause when an exception occurs in a
resolver.

### How I Tested These Changes

added test, screenshot:
<img width="841" alt="Screen Shot 2023-02-09 at 3 17 59 PM"
src="https://user-images.githubusercontent.com/202219/217941619-9b9a9431-1994-4bf7-b92d-7e8cd5a7708a.png">
  • Loading branch information
alangenfeld committed Feb 15, 2023
1 parent 7962972 commit 17ea06c
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 9 deletions.
27 changes: 20 additions & 7 deletions js_modules/dagit/packages/core/src/app/AppError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,20 @@ import * as React from 'react';

import {showCustomAlert} from './CustomAlertProvider';

interface DagsterSerializableErrorInfo {
message: string;
stack: string[];
cls_name: string | null;
cause: DagsterSerializableErrorInfo | null;
context: DagsterSerializableErrorInfo | null;
}

interface DagsterGraphQLError extends GraphQLError {
stack_trace: string[];
cause?: DagsterGraphQLError;
extensions:
| {
errorInfo?: DagsterSerializableErrorInfo;
}
| undefined;
}

const ErrorToaster = Toaster.create({position: 'top-right'});
Expand Down Expand Up @@ -53,24 +64,26 @@ interface AppStackTraceLinkProps {

const AppStackTraceLink = ({error, operationName}: AppStackTraceLinkProps) => {
const title = 'Error';
const stackTraceContent = error.stack_trace ? (
const stackTrace = error?.extensions?.errorInfo?.stack;
const cause = error?.extensions?.errorInfo?.cause;
const stackTraceContent = stackTrace ? (
<>
{'\n\n'}
Stack Trace:
{'\n'}
{error.stack_trace.join('')}
{stackTrace.join('')}
</>
) : null;
const causeContent = error.cause ? (
const causeContent = cause ? (
<>
{'\n'}
The above exception was the direct cause of the following exception:
{'\n\n'}
Message: {error.cause.message}
Message: {cause.message}
{'\n\n'}
Stack Trace:
{'\n'}
{error.cause.stack_trace.join('')}
{cause.stack.join('')}
</>
) : null;
const instructions = (
Expand Down
5 changes: 5 additions & 0 deletions js_modules/dagit/packages/core/src/graphql/schema.graphql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions js_modules/dagit/packages/core/src/graphql/types.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 21 additions & 1 deletion python_modules/dagit/dagit/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
from typing import Any, AsyncGenerator, Dict, List, Optional, Sequence, Tuple, Union, cast

import dagster._check as check
from dagster._serdes import pack_value
from dagster._seven import json
from dagster._utils.error import serializable_error_info_from_exc_info
from dagster_graphql.implementation.utils import ErrorCapture
from graphene import Schema
from graphql import GraphQLError, GraphQLFormattedError
Expand Down Expand Up @@ -66,7 +68,25 @@ def make_request_context(self, conn: HTTPConnection):
...

def handle_graphql_errors(self, errors: Sequence[GraphQLError]):
return [err.formatted for err in errors]
results = []
for err in errors:
fmtd = err.formatted
if err.original_error and err.original_error.__traceback__:
serializable_error = serializable_error_info_from_exc_info(
exc_info=(
type(err.original_error),
err.original_error,
err.original_error.__traceback__,
)
)
fmtd["extensions"] = {
**fmtd.get("extensions", {}),
"errorInfo": pack_value(serializable_error),
}

results.append(fmtd)

return results

async def graphql_http_endpoint(self, request: Request):
"""
Expand Down
18 changes: 17 additions & 1 deletion python_modules/dagit/dagit_tests/webserver/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
op,
)
from dagster._core.events import DagsterEventType
from dagster._serdes import unpack_value
from dagster._seven import json
from dagster._utils.error import SerializableErrorInfo
from dagster_graphql.version import __version__ as dagster_graphql_version
from starlette.testclient import TestClient

Expand Down Expand Up @@ -113,7 +115,9 @@ def test_graphql_get(instance, test_client: TestClient): # pylint: disable=unus
def test_graphql_invalid_json(instance, test_client: TestClient): # pylint: disable=unused-argument
# base case
response = test_client.post(
"/graphql", content='{"query": "foo}', headers={"Content-Type": "application/json"}
"/graphql",
content='{"query": "foo}',
headers={"Content-Type": "application/json"},
)

print(str(response.text))
Expand Down Expand Up @@ -179,6 +183,18 @@ def test_graphql_post(test_client: TestClient):
assert response.status_code == 400, response.text


def test_graphql_error(test_client: TestClient):
response = test_client.post(
"/graphql",
params={"query": "{test{alwaysException}}"},
)
assert response.status_code == 500, response.text
error = response.json()["errors"][0]
serdes_err = error["extensions"]["errorInfo"]
original_err = unpack_value(serdes_err)
assert isinstance(original_err, SerializableErrorInfo)


def test_graphql_ws_error(test_client: TestClient):
# wtf pylint
# pylint: disable=not-context-manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
from ..schedules import GrapheneScheduleOrError, GrapheneSchedulerOrError, GrapheneSchedulesOrError
from ..sensors import GrapheneSensorOrError, GrapheneSensorsOrError
from ..tags import GraphenePipelineTagAndValues
from ..test import GrapheneTestFields
from ..util import ResolveInfo, get_compute_log_manager, non_null_list
from .assets import GrapheneAssetOrError, GrapheneAssetsOrError
from .execution_plan import GrapheneExecutionPlanOrError
Expand Down Expand Up @@ -431,6 +432,11 @@ class Meta:
description="Whether or not the NUX should be shown to the user",
)

test = graphene.Field(
GrapheneTestFields,
description="Provides fields for testing behavior",
)

def resolve_repositoriesOrError(
self,
graphene_info: ResolveInfo,
Expand Down Expand Up @@ -869,3 +875,6 @@ def resolve_capturedLogs(

def resolve_shouldShowNux(self, graphene_info):
return graphene_info.context.instance.nux_enabled and not get_has_seen_nux()

def resolve_test(self, _):
return GrapheneTestFields()
11 changes: 11 additions & 0 deletions python_modules/dagster-graphql/dagster_graphql/schema/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import graphene


class GrapheneTestFields(graphene.ObjectType):
class Meta:
name = "TestFields"

alwaysException = graphene.String()

def resolve_alwaysException(self, _):
raise Exception("as advertised")

0 comments on commit 17ea06c

Please sign in to comment.