Skip to content

fix(apns): handling of the certificate expiration error#364

Merged
geekbrother merged 2 commits intomainfrom
fix/handle_cert_expiration
May 29, 2025
Merged

fix(apns): handling of the certificate expiration error#364
geekbrother merged 2 commits intomainfrom
fix/handle_cert_expiration

Conversation

@geekbrother
Copy link
Contributor

Description

This PR adds proper handling of the received fatal alert: CertificateExpired APNS error.

How Has This Been Tested?

Not tested.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@geekbrother geekbrother self-assigned this May 20, 2025
// Note: Should we pass this error back up?
Err(_e) => None,
};
let public_ip = networking::find_public_ip_addr().ok();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixing clippy error.

Choose a reason for hiding this comment

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

Fix

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces handling for APNS CertificateExpired errors and simplifies public IP lookup.

  • Simplify public_ip assignment by using .ok()
  • Add a new match arm for CertificateExpired in the APNS provider

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/state.rs Replace manual match with .ok() for public IP
src/providers/apns.rs Add explicit handling for CertificateExpired alerts
Comments suppressed due to low confidence (3)

src/providers/apns.rs:178

  • [nitpick] Using dbg as a variable name can be confused with the dbg! macro. Rename this binding to something more descriptive, like debug_msg.
dbg if dbg.contains("received fatal alert: CertificateExpired") => {

src/providers/apns.rs:175

  • New error variant ApnsCertificateExpired is introduced but there are no tests for this branch. Consider adding a unit test that simulates the CertificateExpired response.
impl PushProvider for ApnsProvider {

src/providers/apns.rs:179

  • The client_error identifier isn't in scope here and named placeholders aren't supported by info!. You likely need to reference the correct error variable and use {} formatting with positional arguments.
info!("APNs certificate expired: debug:{dbg}, display: {client_error}");

@geekbrother geekbrother merged commit cf976f3 into main May 29, 2025
5 of 11 checks passed
@geekbrother geekbrother deleted the fix/handle_cert_expiration branch May 29, 2025 11:05
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.

5 participants