search

diffix / explorer

Tool to automatically explore and generate stats on data anonymized using Diffix
MIT License
2 stars 1 forks source link

Catch and propagate exceptions from the explorer #51

Closed dandanlen closed 4 years ago

dandanlen commented 4 years ago

Currently, an exception that causes the explorer to fail doesn't automatically trigger an "error" response to the explorer client - some just fail silently (from the api consumer's point of view).

We need to ensure that any internal error causes an "error" response on the API, as well as logging the detailed error somewhere so it can be investigated.

We should try to use appropriate Http response codes for all errors (for example, "500: Internal Server Error" for almost all exceptions thrown from the explorer).

dandanlen commented 4 years ago

https://docs.microsoft.com/en-us/aspnet/core/web-api/handle-errors?view=aspnetcore-3.1

This page has details about how to set up catching and propagating exceptions correctly in ASP.NET. Mainly for future reference - For now this can be considered low-priority.

dandanlen commented 4 years ago

This issue is relevant to #99

dandanlen commented 4 years ago

Another thing to note - an exception in a subtask or query should not automatically cause a Http 500 response. Currently, most exceptions bubble up to the surface and cause the entire request to transition to a failed state. Instead, errors should be logged on the server and (or?) notified to the client as part of the exploration result.

AndreiBozantan commented 4 years ago

One problem, for example, is that when using an invalid API URL (e.g. https://attack.aircloak.com which is missing the /api part at the end) the exception thrown is somewhat vague.

dandanlen commented 4 years ago

Linking the following discussion here since it seems to be relevant: https://github.com/dotnet/runtime/issues/31494

dandanlen commented 4 years ago

This has largely been addressed by the integration of Sentry and definition of specific ApiException types in the Aircloak Json interface. Closing since the issue is rather vague - if any more specific issues come up, they can raised separately.