HttpException stacktrace is lost when ClientException is thrown

[maks]

When a ClientException is thrown from here in io_client.dart, it does not preserve the original HttpException's stacktrace (because of this still open issue there is no way to preserve the original stacktrace if not rethrowing the original exception).

This makes it almost impossible to debug issues such as this one when the exceptions are being reported via crash analytics and are not easily reproducible under a debugger.

[natebosch]

One possible solution is to add a StackTrace field on ClientException.

cc @lrhn what do you think?

I think we'd need to include it in the toString(). That has some negative implications for users that don't care to see it.

[maks]

Thanks @natebosch that would be great if it could be added as an extra field!

My opinion would be: I suspect anyone already doing using toString() on a ClientException would not be unhappy for more context on the exception too.

[jamotaylor]

This might lead to less people posting issues about CORS errors 😄

[natebosch]

We have Error.throwWithStackTrace now. I wonder if it would be more useful to use that so the except appears to come from the original location.

@brianquinlan - do you have any thoughts? It might make diagnosing bugs in this package harder, but should make bugs in client code that causes exception in other places easier to solve. Using a separate field could preserve both stacks and be useful for both use cases, but also retains a lot of noise.

[natebosch]

cc @lrhn as well for thoughts on whether wrapping an exception in a consistent class is a valid use case for throwWithStackTrace.

[lrhn]

It does worry me a little, conceptually.

If you catch an exception and handle it (by wrapping), you seem to be at an abstraction boundary. You are changing the messaging to an exception from your own API. Why does the client need to know what happens below that boundary? Does it really matter? What can they use it for? An exception is not something you should need to debug, it's not an error. You shouldn't care about the stack trace of an exception any more than yous should care about the stack trace leading to a return value.

If the originally thrown object is an Error, it makes sense to say where it came from, but then the error object itself would already carry the original stack trace in Error.stackTrace, and I'd not wrapping it at all.

When it's an Exception, then you should ignore stack traces. The rethrown value itself should contain all the information the client needs to handle the exceptional occurrence. The implementation details of how the exception occurred shouldn't matter.

However, the API already exists and handles exceptions by wrapping, and users seem to care about the stack trace of their exception, even if they shouldn't. Also, some exceptions should probably have been errors originally, which muddies the water.

In that case, rethrowing a wrapped object with the original stack trace is probably reasonable. Just hope that the user won't get confused by seeing a ClientException, checking the stack trace, then look at the code that should have thrown it and see a throw SomeOtherException. (But then, they shouldn't, because they should not be debugging exceptions, they are not bugs.)

If you're not squeamish, you can combine all the stack traces:

catch (e, s) {
  Error.throwWithStackTrace(ClientException(e), StackTrace.fromString(
    "${StackTrace.current}\n-- Originally: --\n$s"),
  );
}

(But that always risks tripping up code which parses stack traces.)

[brianquinlan]

I think that preserving the original stack trace is probably going to be more useful in practice.

[longtimedeveloper]

I lost a bunch of time trying to figure out this strange exception message. Well, actually no understandable message at all. No stack trace. No information at all.

Message: "XMLHttpRequest error."

ClientException: XMLHttpRequest error., uri=http://localhost:7071/api/Login

This was a CORS rejection. I have corrected the CORS settings on the server.

Why doesn't the ClientException at least least have a reasonable message like "most exceptions"

This is a bug and must be corrected. Almost 3 years old???

So... do developers assume that all ClientException's are CORS violations?

Thank you.

[natebosch]

Why doesn't the ClientException at least least have a reasonable message

This is a limitation of the browser - the information is not available anywhere other than in the console message. We cannot expose this information in the Dart code because the browser does not expose it to the JS code.

See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS/Errors#:~:text=For%20security%20reasons%2C%20specifics%20about%20what%20went%20wrong%20with%20a%20CORS%20request%20are%20not%20available%20to%20JavaScript%20code