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

DioException.toString() should contain the request information #1949

Open
iota9star opened this issue Sep 1, 2023 · 9 comments · May be fixed by #2296 or #2297
Open

DioException.toString() should contain the request information #1949

iota9star opened this issue Sep 1, 2023 · 9 comments · May be fixed by #2296 or #2297
Labels
s: feature This issue indicates a feature request

Comments

@iota9star
Copy link

iota9star commented Sep 1, 2023

Request Statement

When printing exception information, it is impossible to quickly locate the source of the request. Here are some examples:

DioException [connection timeout]: The request connection took longer than 0:00:30.000000. It was aborted.

The exception message only shows that a connection timeout occurred, but does not provide any information about which request timed out. This makes it difficult to quickly identify the problematic request when debugging.

Some additional context that would be useful:

The URL of the request
The HTTP method (GET, POST, etc)
Any identifiers like request ID
So a more helpful exception could be:

DioException [connection timeout]: GET https://example.com/api/v1/users took longer than 30s and was aborted.

By including details like the URL and request method in the exception, it becomes much faster to pinpoint where the problem is occurring.

Solution Brainstorm

No response

@iota9star iota9star added the s: feature This issue indicates a feature request label Sep 1, 2023
@kuhnroyal
Copy link
Member

The request information is available via exception.requestOptions. You can build your own extension function to print a customized message.

@iota9star
Copy link
Author

The requestOptions field is useful, but not relevant to the actual error message.

A friendly error message,toString method should contain concise, accurate and effective information. For example, now I have a method that contains some Dio requests, and some other methods that may also throw errors. Normally, I only need to call print to meet the need when I want to print the exception information, but now I need to first judge if the current error is a DioException, then extract useful information from it, and customize the message according to the error type. This obviously introduces a lot of burden. Of course, I can also write an extension method to unify custom message assembly to reduce boilerplate code, but this obviously has a huge mental overhead when I have many methods.

@kuhnroyal kuhnroyal changed the title DioException should contain the request information DioException.toString() should contain the request information Sep 3, 2023
@ueman
Copy link
Contributor

ueman commented Sep 6, 2023

@iota9star Are you willing to contribute this as a PR?

@kuhnroyal
Copy link
Member

Can we always print the URL, are there any possible privacy concerns if we omit query parameters?

@iota9star
Copy link
Author

Appropriately outputting additional information does not pose much of a problem. Request method, request path and such do not have major privacy issues. Error messages are controlled by developers, and leaks are only individual developer's problems. Of course we can also try printing exception information at different levels through configuration.

@iota9star
Copy link
Author

@iota9star Are you willing to contribute this as a PR?

This is too difficult for me.

@SpeedReach
Copy link
Contributor

SpeedReach commented Sep 30, 2023

Doesn't writing your own error log interceptor allow this?

/// Called when an exception was occurred during the request.
void onError(
DioException err,
ErrorInterceptorHandler handler,
) {
handler.next(err);
}

@nottmey
Copy link

nottmey commented Feb 15, 2024

@ueman Hi Jonas, would this be a valid approach? Any concerns?

class ErrorMessageRewritingInterceptor extends Interceptor {
  @override
  void onError(DioException err, ErrorInterceptorHandler handler) {
    final errWithHelpfulMessage =
        err.copyWith(message: '<some helpful message for debugging>');
    super.onError(errWithHelpfulMessage, handler);
  }
}

@AlexV525
Copy link
Member

I've filed 2 PRs to find out if there is a better answer for the issue. Please vote or add comments if you'd like to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: feature This issue indicates a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants