Skip to content

Commit 354b4e7

Browse files
Copilotbbockelm
andcommitted
Refactor monitoring code per code review feedback
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
1 parent 2d07f02 commit 354b4e7

File tree

1 file changed

+46
-40
lines changed

1 file changed

+46
-40
lines changed

src/scitokens_internal.h

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -495,32 +495,20 @@ class Validator {
495495

496496
result = verify_async_continue(std::move(result));
497497
}
498-
} catch (const std::exception &e) {
499-
// Record failure if we have an issuer
498+
499+
// Record successful validation
500500
if (has_issuer) {
501501
auto end_time = std::chrono::steady_clock::now();
502502
auto duration = std::chrono::duration_cast<std::chrono::nanoseconds>(
503503
end_time - start_time);
504-
505-
std::string error_msg = e.what();
506-
507-
// Check if this is a failed issuer lookup (network/DNS errors)
508-
if (error_msg.find("resolve") != std::string::npos ||
509-
error_msg.find("host") != std::string::npos ||
510-
error_msg.find("network") != std::string::npos ||
511-
error_msg.find("Failed to retrieve") != std::string::npos) {
512-
internal::MonitoringStats::instance().record_failed_issuer_lookup(issuer);
513-
}
514-
515-
// Check if this is an expiration error
516-
if (error_msg.find("exp") != std::string::npos ||
517-
error_msg.find("expir") != std::string::npos) {
518-
internal::MonitoringStats::instance().record_expired_token(issuer);
519-
}
520-
521-
internal::MonitoringStats::instance().record_validation_failure(
504+
internal::MonitoringStats::instance().record_validation_success(
522505
issuer, duration.count());
523506
}
507+
} catch (const std::exception &e) {
508+
// Record failure if we have an issuer
509+
if (has_issuer) {
510+
record_validation_error(issuer, e, start_time);
511+
}
524512
throw;
525513
}
526514
}
@@ -541,32 +529,20 @@ class Validator {
541529
while (!result->m_done) {
542530
result = verify_async_continue(std::move(result));
543531
}
544-
} catch (const std::exception &e) {
545-
// Record failure if we have an issuer
532+
533+
// Record successful validation
546534
if (has_issuer) {
547535
auto end_time = std::chrono::steady_clock::now();
548536
auto duration = std::chrono::duration_cast<std::chrono::nanoseconds>(
549537
end_time - start_time);
550-
551-
std::string error_msg = e.what();
552-
553-
// Check if this is a failed issuer lookup (network/DNS errors)
554-
if (error_msg.find("resolve") != std::string::npos ||
555-
error_msg.find("host") != std::string::npos ||
556-
error_msg.find("network") != std::string::npos ||
557-
error_msg.find("Failed to retrieve") != std::string::npos) {
558-
internal::MonitoringStats::instance().record_failed_issuer_lookup(issuer);
559-
}
560-
561-
// Check if this is an expiration error
562-
if (error_msg.find("exp") != std::string::npos ||
563-
error_msg.find("expir") != std::string::npos) {
564-
internal::MonitoringStats::instance().record_expired_token(issuer);
565-
}
566-
567-
internal::MonitoringStats::instance().record_validation_failure(
538+
internal::MonitoringStats::instance().record_validation_success(
568539
issuer, duration.count());
569540
}
541+
} catch (const std::exception &e) {
542+
// Record failure if we have an issuer
543+
if (has_issuer) {
544+
record_validation_error(issuer, e, start_time);
545+
}
570546
throw;
571547
}
572548
}
@@ -947,6 +923,36 @@ class Validator {
947923
}
948924
}
949925

926+
/**
927+
* Helper method to record monitoring statistics for validation errors.
928+
*/
929+
void record_validation_error(const std::string &issuer,
930+
const std::exception &e,
931+
const std::chrono::steady_clock::time_point &start_time) {
932+
auto end_time = std::chrono::steady_clock::now();
933+
auto duration = std::chrono::duration_cast<std::chrono::nanoseconds>(
934+
end_time - start_time);
935+
936+
std::string error_msg = e.what();
937+
938+
// Check if this is a failed issuer lookup (network/DNS errors)
939+
if (error_msg.find("resolve") != std::string::npos ||
940+
error_msg.find("host") != std::string::npos ||
941+
error_msg.find("network") != std::string::npos ||
942+
error_msg.find("Failed to retrieve") != std::string::npos) {
943+
internal::MonitoringStats::instance().record_failed_issuer_lookup(issuer);
944+
}
945+
946+
// Check if this is an expiration error
947+
if (error_msg.find("exp") != std::string::npos ||
948+
error_msg.find("expir") != std::string::npos) {
949+
internal::MonitoringStats::instance().record_expired_token(issuer);
950+
}
951+
952+
internal::MonitoringStats::instance().record_validation_failure(
953+
issuer, duration.count());
954+
}
955+
950956
bool m_validate_all_claims{true};
951957
SciToken::Profile m_profile{SciToken::Profile::COMPAT};
952958
SciToken::Profile m_validate_profile{SciToken::Profile::COMPAT};

0 commit comments

Comments
 (0)