To hell with get_stacktrace! #2494

NelsonVides · 2019-10-16T13:33:06Z

Now only some dependencies are still giving that silly get_stacktrace/0 deprecated warning. To be precise, exactly these dependencies (and dependencies of dependencies and so on):

worker_pool
katana
proper
pooler
cqerl
eper
nklib
nkpacket
fusco
exometer_core

~~Testing on CircleCI is going to be a ton faster than locally~~ 😛

codecov · 2019-10-16T14:05:45Z

Codecov Report

Merging #2494 into master will increase coverage by 0.01%.
The diff coverage is 13.63%.

@@            Coverage Diff             @@
##           master    #2494      +/-   ##
==========================================
+ Coverage   18.07%   18.09%   +0.01%     
==========================================
  Files         340      340              
  Lines       29369    29329      -40     
==========================================
- Hits         5309     5307       -2     
+ Misses      24060    24022      -38

Impacted Files	Coverage Δ
src/wpool/mongoose_wpool.erl	`59.57% <ø> (+0.62%)`	⬆️
src/ejabberd_ctl.erl	`0.64% <ø> (ø)`	⬆️
src/pubsub/mod_pubsub.erl	`0% <ø> (ø)`	⬆️
src/gen_iq_handler.erl	`29.54% <ø> (ø)`	⬆️
src/mongoose_iq.erl	`65.38% <ø> (+2.42%)`	⬆️
src/mod_roster.erl	`21.49% <ø> (+0.05%)`	⬆️
src/mongoose_api_admin.erl	`75.43% <ø> (ø)`	⬆️
src/mongoose_commands.erl	`93.1% <ø> (ø)`	⬆️
src/mam/mod_mam_rdbms_arch.erl	`0% <ø> (ø)`	⬆️
src/mongoose_api_common.erl	`73.59% <ø> (+0.94%)`	⬆️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52a84bc...2c3d061. Read the comment docs.

mongoose-im · 2019-10-17T00:54:23Z

7331.1 / Erlang 22.0 / small_tests / faa861a
Reports root / small

7331.2 / Erlang 22.0 / internal_mnesia / faa861a
Reports root/ big
OK: 1267 / Failed: 0 / User-skipped: 136 / Auto-skipped: 0

7331.3 / Erlang 22.0 / odbc_mssql_mnesia / faa861a
Reports root/ big
OK: 3181 / Failed: 0 / User-skipped: 262 / Auto-skipped: 0

7331.4 / Erlang 22.0 / mysql_redis / faa861a
Reports root/ big
OK: 3176 / Failed: 0 / User-skipped: 267 / Auto-skipped: 0

7331.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / faa861a
Reports root/ big
OK: 514 / Failed: 0 / User-skipped: 32 / Auto-skipped: 0

7331.5 / Erlang 22.0 / riak_mnesia / faa861a
Reports root/ big
OK: 1493 / Failed: 0 / User-skipped: 145 / Auto-skipped: 0

7331.6 / Erlang 22.0 / ldap_mnesia / faa861a
Reports root/ big
OK: 1236 / Failed: 0 / User-skipped: 167 / Auto-skipped: 0

7331.9 / Erlang 21.3 / pgsql_mnesia / faa861a
Reports root/ big / small
OK: 3194 / Failed: 0 / User-skipped: 249 / Auto-skipped: 0

michalwski

Thanks for the effort! This was quick :) I have only one cosmetic comment to the code. Also there is a small inconsistency in the way variable containing the stack trace is called. Sometimes it's Stacktrace and sometimes StackTrace. Do you think this naming can be made consistent or is there a reason to have both variants?

michalwski · 2019-10-17T07:28:55Z

src/rdbms/mongoose_rdbms.erl

+                           "reason=~p:~p stacktrace=~1000p",
+                           [Class, Other, Stacktrace]),
+                {{'EXIT', Other},Stacktrace}
+        end,


A cosmetic thing. I think readability of this block could be improved a bit by moving they try ... catch clause to a dedicated function like apply_transaction_function. What do you think?

mongoose-im · 2019-10-17T07:39:24Z

7333.1 / Erlang 22.0 / small_tests / 8a8a103
Reports root / small

7333.2 / Erlang 22.0 / internal_mnesia / 8a8a103
Reports root/ big
OK: 1267 / Failed: 0 / User-skipped: 136 / Auto-skipped: 0

7333.3 / Erlang 22.0 / odbc_mssql_mnesia / 8a8a103
Reports root/ big
OK: 3181 / Failed: 0 / User-skipped: 262 / Auto-skipped: 0

7333.4 / Erlang 22.0 / mysql_redis / 8a8a103
Reports root/ big
OK: 3176 / Failed: 0 / User-skipped: 267 / Auto-skipped: 0

7333.5 / Erlang 22.0 / riak_mnesia / 8a8a103
Reports root/ big
OK: 1493 / Failed: 0 / User-skipped: 145 / Auto-skipped: 0

7333.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / 8a8a103
Reports root/ big
OK: 514 / Failed: 0 / User-skipped: 32 / Auto-skipped: 0

7333.6 / Erlang 22.0 / ldap_mnesia / 8a8a103
Reports root/ big
OK: 1236 / Failed: 0 / User-skipped: 167 / Auto-skipped: 0

7333.9 / Erlang 21.3 / pgsql_mnesia / 8a8a103
Reports root/ big / small
OK: 3194 / Failed: 0 / User-skipped: 249 / Auto-skipped: 0

NelsonVides · 2019-10-17T07:59:45Z

@michalwski

Thanks for the effort! This was quick :) I have only one cosmetic comment to the code. Also there is a small inconsistency in the way variable containing the stack trace is called. Sometimes it's Stacktrace and sometimes StackTrace. Do you think this naming can be made consistent or is there a reason to have both variants?

I thought the same while doing this, but I pretty much followed the style of the code already present. Sometimes we had pattern matching against C:R and sometimes against Class:Reason, sometimes the stacktrace was called StackTrace and some others it was Stacktrace... I didn't know against which to unify. But I'm gonna try StackTrace just for the fact that that is actually two glued words and not one alone. I can easily grep over that together with the cosmetic comment 🙂

mongoose-im · 2019-10-17T08:53:50Z

7334.1 / Erlang 22.0 / small_tests / a3ddf3b
Reports root / small

7334.2 / Erlang 22.0 / internal_mnesia / a3ddf3b
Reports root/ big
OK: 1267 / Failed: 0 / User-skipped: 136 / Auto-skipped: 0

7334.3 / Erlang 22.0 / odbc_mssql_mnesia / a3ddf3b
Reports root/ big
OK: 2484 / Failed: 0 / User-skipped: 199 / Auto-skipped: 0

7334.4 / Erlang 22.0 / mysql_redis / a3ddf3b
Reports root/ big
OK: 2479 / Failed: 0 / User-skipped: 204 / Auto-skipped: 0

7334.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / a3ddf3b
Reports root/ big
OK: 328 / Failed: 0 / User-skipped: 28 / Auto-skipped: 0

7334.5 / Erlang 22.0 / riak_mnesia / a3ddf3b
Reports root/ big
OK: 1398 / Failed: 0 / User-skipped: 145 / Auto-skipped: 0

7334.6 / Erlang 22.0 / ldap_mnesia / a3ddf3b
Reports root/ big
OK: 1236 / Failed: 0 / User-skipped: 167 / Auto-skipped: 0

7334.9 / Erlang 21.3 / pgsql_mnesia / a3ddf3b
Reports root/ big / small
OK: 2497 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

Also unify the stacktrace variable naming to `StackTrace`, with camelcase, for the fact that this is actually a two-words variable. This applies only on the case when we pattern-match against `Class:Reason:StackTrace`. The other option is against `C:R:S`. This style was preserved as it was.

NelsonVides added WIP 🚧 waiting-for-ci labels Oct 16, 2019

NelsonVides force-pushed the get_stacktrace branch 3 times, most recently from 589ac3a to 47858f8 Compare October 16, 2019 14:05

NelsonVides force-pushed the get_stacktrace branch from 47858f8 to 89acb5d Compare October 16, 2019 14:49

NelsonVides added waiting-for-review and removed WIP 🚧 labels Oct 16, 2019

This comment has been minimized.

Sign in to view

NelsonVides force-pushed the get_stacktrace branch from 8c31e7b to a5e3401 Compare October 16, 2019 21:17

NelsonVides requested review from fenek and michalwski October 17, 2019 06:36

NelsonVides removed the waiting-for-ci label Oct 17, 2019

NelsonVides force-pushed the get_stacktrace branch from a5e3401 to 6030a7b Compare October 17, 2019 06:40

michalwski reviewed Oct 17, 2019

View reviewed changes

NelsonVides force-pushed the get_stacktrace branch from 6030a7b to 2167198 Compare October 17, 2019 08:25

NelsonVides requested a review from michalwski October 17, 2019 08:26

michalwski approved these changes Oct 17, 2019

View reviewed changes

NelsonVides added 3 commits October 17, 2019 11:54

Upgraded rebar_faster_deps

c99576b

Extract the try-catch to a separate function

2c3d061

NelsonVides force-pushed the get_stacktrace branch from 2167198 to 2c3d061 Compare October 17, 2019 09:54

michalwski merged commit 4a3a558 into master Oct 17, 2019

michalwski deleted the get_stacktrace branch October 17, 2019 10:50

michalwski added this to the 3.6.0 milestone Jan 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

To hell with get_stacktrace! #2494

To hell with get_stacktrace! #2494

NelsonVides commented Oct 16, 2019 •

edited

Loading

codecov bot commented Oct 16, 2019 •

edited

Loading

This comment has been minimized.

mongoose-im commented Oct 17, 2019 •

edited

Loading

michalwski left a comment

michalwski Oct 17, 2019

mongoose-im commented Oct 17, 2019 •

edited

Loading

NelsonVides commented Oct 17, 2019 •

edited

Loading

mongoose-im commented Oct 17, 2019 •

edited

Loading

To hell with get_stacktrace! #2494

To hell with get_stacktrace! #2494

Conversation

NelsonVides commented Oct 16, 2019 • edited Loading

codecov bot commented Oct 16, 2019 • edited Loading

Codecov Report

This comment has been minimized.

mongoose-im commented Oct 17, 2019 • edited Loading

michalwski left a comment

Choose a reason for hiding this comment

michalwski Oct 17, 2019

Choose a reason for hiding this comment

mongoose-im commented Oct 17, 2019 • edited Loading

NelsonVides commented Oct 17, 2019 • edited Loading

mongoose-im commented Oct 17, 2019 • edited Loading

NelsonVides commented Oct 16, 2019 •

edited

Loading

codecov bot commented Oct 16, 2019 •

edited

Loading

mongoose-im commented Oct 17, 2019 •

edited

Loading

mongoose-im commented Oct 17, 2019 •

edited

Loading

NelsonVides commented Oct 17, 2019 •

edited

Loading

mongoose-im commented Oct 17, 2019 •

edited

Loading