MihaelIsaev / FCM

⚡️ PushNotifications through FireBase for Vapor 3 and 4.
MIT License
120 stars 33 forks source link

Warning when configuration FCM: `Cannot modify client configuration after client has been used` #35

Closed sroebert closed 2 years ago

sroebert commented 2 years ago

When setting the configuration of FCM, I get the warning from Vapor: Cannot modify client configuration after client has been used..

This happens because FCM is trying to set application.http.client.configuration.ignoreUncleanSSLShutdown to true after using the application.client property in the following code:

public init(application: Application) {
    self.init(application: application, client: application.client)
}

I'm wondering, does this configuration really need to be set to true in the first place (as there is no documentation on why this is done)? If it is needed, this code probably needs to be moved to the public init with the application, to avoid the warning and actually setting the value to true.

MihaelIsaev commented 2 years ago

That's the good question sir. As far as I remember Google servers always do uncleanSSLShutdown and we had problems with requests to Firebase, that's why this setting appeared in the lib.

You could scroll that old chat a bit just to read that nobody knows why Google won't do clean shutdown..

If it is needed, this code probably needs to be moved to the public init with the application

I tried to move app.fcm.configuration = ... to the top of my configure.swift and I still see the warning. Then I set a breakpoint in Vapor sources at line 45 and debugger never stopped on it, so it seems that HTTP configuration is set somewhere in the very beginning under the hood and it is impossible to change it from the outside later.

Maybe Google started doing clean ssl shutdown? I tried this request today

app.client.get("https://www.google.com").whenComplete {
    switch $0 {
    case .failure(let error): print("google.com error: \(error)")
    case .success: print("google.com success")
    }
}

and it prints google.com success so it seems clean ssl shutdown is here 🎉

And it seems that the lib works well for the very long time already and I have no claims from the users and myself as well.

So it means that we could just delete code at lines 38-40 in FCM.swift since it doesn't do any job anyway.

@sroebert would you like to send pull request?

sroebert commented 2 years ago

I fixed it in my code for now, by already setting the value before configuring FCM:

app.http.client.configuration.ignoreUncleanSSLShutdown = true
app.fcm.configuration = ...

But I can definitely make a PR for removing this yes: #36