Skip to content

Conversation

@maximvl
Copy link

@maximvl maximvl commented Sep 13, 2019

Hi, the idea here is to allow adapters to return stacktrace on transaction errors as this simplifies debugging a lot. The caller can control it via transaction Opts second parameter, something like [{return_stacktrace, true}]

@maximvl maximvl force-pushed the add_transaction_stacktrace branch from 717b603 to d3d75b5 Compare September 13, 2019 09:20
language: erlang
otp_release:
- 20.2
- 20.3
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, let's add -21.3 too, or just replace it.

@cabol
Copy link
Owner

cabol commented Sep 13, 2019

@maximvl I think I disagree with this, what API function returns the stack trace? That is something it should be handled by the caller.

@maximvl
Copy link
Author

maximvl commented Sep 13, 2019

@cabol it's up to an adapter to implement this feature, but if there is an error inside transaction we only return the message which is most of the time something like badmatch and impossible to debug.

Fun :: fun(() -> any()),
Opts :: xdb_lib:keyword(),
Res :: {ok, any()} | {error, any()}.
Res :: {ok, any()} | {error, any()} | {error, any(), [tuple()]}.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximvl according to your thought it's up to an adapter to implement this feature, I think we shouldn't modify this spec, {error, any()} should be enough, you can pass the ST like {error, {Reason, ST}}, again, it is up to the adapter right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cabol in a way yes, but the problem is the original error might look like {Reason, Data} and then it becomes hard to know for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants