crypto: add internal error codes#37650
Conversation
nodejs-github-bot
added
c++
RaisinTen
force-pushed
the
crypto/add-internal-error-codes
branch
2 times, most recently
from
8bdf37e to
703bbf0
Compare
src/crypto/crypto_util.cc
Outdated
| } | ||
|
|
||
| template<typename... Args> | ||
| void CryptoErrorStore::Insert(const NodeCryptoError error, Args&&... args) { |
There was a problem hiding this comment.
This might need to go into a crypto_util-inl.h file at some point (because template functions are implicitly also inline functions), but if this works right now, it should be fine 👍
There was a problem hiding this comment.
@addaleax I don't think we have a crypto_util-inl.h.
I did find -name crypto_util-inl.h and couldn't find it. 👀
There was a problem hiding this comment.
Yeah, sorry for being unclear – this would be the first function in that file :)
There was a problem hiding this comment.
@addaleax Moved it to src/crypto/crypto_util-inl.h. I noticed a couple of other template functions and classes in src/crypto/crypto_util.h. Should we move those to src/crypto/crypto_util-inl.h as well (in another PR)?
Also,
../src/crypto/crypto_util.h:351:14: error: no matching member function for call to 'TrackField'
tracker->TrackField("errors", errors_);
~~~~~~~~~^~~~~~~~~~To fix this, should I again change CryptoErrorStore to inherit from std::vector<std::string> or is there another way?
RaisinTen
force-pushed
the
crypto/add-internal-error-codes
branch
2 times, most recently
from
5231d3d to
4ff9e88
Compare
RaisinTen
added
the
author ready
PR-URL: nodejs#37650 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95
force-pushed
the
crypto/add-internal-error-codes
branch
from
6ff54c7 to
a28cb93
Compare
|
Landed in a28cb93 |
PR-URL: #37650 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #37650 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
6 participants
According to addaleax's comment here: #37555 (review)