mikebridge commented on PR #45537:
URL: https://github.com/apache/superset/pull/45537#issuecomment-6321453932
@fitzee thanks, both logging points were right. Fixed in
cee77a383a6ae4f3bd4aef6bcfc0e34b1ab1f6d1. Status codes and response text
haven't changed: every case below still returns the same generic 500, with no
chained cause.
**1. The discovery log now identifies the view.** Each fault is logged as
one record with `operation`, `view_id`, `layer_uuid` and the real exception
type, followed by the file, line and function of the calling frames and of the
fault's own frames. So a dashboard failure now names the view and the
`ValueError` on the same line, instead of `error=SemanticLayerExecutionError`
with the real type on a separate line. For example:
```
Semantic provider metadata discovery failed: operation=get_metrics view_id=7
layer_uuid=8765…8765 error=ValueError
Called from:
File ".../superset/semantic_layers/models.py", line 713, in data
...
Traceback (exception text and source omitted):
File ".../provider/views.py", line 42, in get_metrics
```
I left out source lines as well as the message. `format_tb` prints each
frame's source line, and a source line can hold a string literal, so the record
keeps only frame locations.
`test_dashboard_datasets_provider_discovery_fault_logs_no_detail` now asserts
that the operation, view id, layer uuid and type are all on the same record.
**2. Superset faults are logged separately from provider faults.** I kept
binding inside the wrapper, so provider construction faults stay classified.
The calls that actually enter provider code (`from_configuration`,
`get_semantic_view`, `get_metrics`, `get_dimensions`) are now marked, and only
exceptions raised from those are logged as provider faults with frames only.
Anything else in that path is logged as `Semantic view metadata discovery
failed in Superset, not the provider: …` with its full traceback. That covers
the metadata-DB lazy load, the registry lookup, `json.loads` of the stored
configuration, and the generation capture. The traceback is built without the
exception chain, so a provider exception that Superset code was handling when
it failed is still not printed.
There are tests for a corrupt stored configuration (`JSONDecodeError`), an
unregistered layer type (`KeyError`), each provider binding stage, and a
Superset fault raised while handling a provider exception. If a provider
exception refuses the marker attribute, it propagates unchanged.
**Consistency with #45531.** With (1) in place, the MCP handlers there still
get the generic error from these reads. The server log now has the view, layer,
operation, type and stack, but still not the provider's message. That is the
remaining difference from "detailed diagnostics remain in server logs", and
query execution (`execute_semantic_query`) still logs the full provider
exception. I'd rather settle which of those two policies wins in one place than
drift in both PRs. If you're happy with frames-without-message for discovery,
I'll word #45531 to match.
I also corrected the PR description. The MCP and structure-endpoint reads
you pointed out don't go through these helpers. The description now says so
instead of claiming they run "under their own error handling" as if they were
unaffected.
Your test findings (the four surviving mutants) and the nits aren't in this
push. I'll pick them up separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
