Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DioError extractor extracts wrong stacktrace #1322

Closed
ueman opened this issue Mar 9, 2023 · 8 comments · Fixed by #1335
Closed

DioError extractor extracts wrong stacktrace #1322

ueman opened this issue Mar 9, 2023 · 8 comments · Fixed by #1335
Labels

Comments

@ueman
Copy link
Collaborator

ueman commented Mar 9, 2023

I have tested this change excessively over the last few days and the whole update from Sentry 6.x/Dio 4.x to Sentry 7.0/Dio 5.0 is a big step back regarding traceability of the DioError. This is in big part due to changes in dio which removed the whole source stacktrace. But due to the DioErrorExtractor there are now also cases where the inner stacktrace of DioError.error.stacktrace is completely missing in the event.

I think the DioError.stacktrace needs to be added by default, as it was the case in 6.x and only the inner cause error/stacktrace should be extracted via the DioErrorExtractor.

Something like:

class DioErrorExtractor extends ExceptionCauseExtractor<DioError> {
  @override
  ExceptionCause? cause(DioError error) {
    if (error.error == null) {
      return null;
    }
    return ExceptionCause(
      error.error,
      error.error is Error ? (error.error as Error).stackTrace : null,
    );
  }
}

Originally posted by @kuhnroyal in #1282 (comment)

@ueman
Copy link
Collaborator Author

ueman commented Mar 9, 2023

@kuhnroyal PR incoming? 😄

@kuhnroyal
Copy link
Contributor

Yea sure but I think we need to solve the dio problem first, otherwise the DioError.stackTrace is almost always null.

@marandaneto
Copy link
Contributor

@denrase can you take a look?
If the stackTrace is null, we can check if error is an Error and use the https://api.dart.dev/stable/2.19.4/dart-core/Error/stackTrace.html as mentioned.
Sounds like the right thing to do.
What if stackTrace isn't null but error also has the error.stackTrace, which one has precedence? I believe the original stackTrace.

@kuhnroyal
Copy link
Contributor

What if stackTrace isn't null but error also has the error.stackTrace, which one has precedence? I believe the original stackTrace.

From my understanding, the DioError. stackTrace should always be on the original error and has nothing to do with the cause being extracted here.

@kuhnroyal
Copy link
Contributor

kuhnroyal commented Mar 9, 2023

Here is an example, the issue is named String DioError inner stacktrace.
The stacktrace belongs to the original error which was extracted via SentryDioClientAdapter - the error simply has no cause.

Bildschirm­foto 2023-03-09 um 21 47 31
Bildschirm­foto 2023-03-09 um 21 47 47

@kuhnroyal
Copy link
Contributor

I think we are missing the original stacktrace here https://github.com/getsentry/sentry-dart/blob/v7.0.0/dio/lib/src/failed_request_interceptor.dart#L42

@kuhnroyal
Copy link
Contributor

Hacked up a basic idea that should solve this, didn't test it yet.

@marandaneto
Copy link
Contributor

I think we are missing the original stacktrace here v7.0.0/dio/lib/src/failed_request_interceptor.dart#L42

In this case, the DioErrorExtractor will do it automatically, its a matter of fixing DioErrorExtractor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants