Rename and update redirect header for QuickPulse #997
Conversation
Library/QuickPulseSender.ts
Outdated
| @@ -114,7 +115,8 @@ class QuickPulseSender { | |||
| const req = https.request(options, (res: http.IncomingMessage) => { | |||
| if (res.statusCode == 200) { | |||
| const shouldPOSTData = res.headers[QuickPulseConfig.subscribed] === "true"; | |||
| const redirectHeader = res.headers[QuickPulseConfig.endpointRedirect] ? res.headers[QuickPulseConfig.endpointRedirect].toString() : null; | |||
| const redirectHeader = res.headers[QuickPulseConfig.endpointRedirect] ? new url.URL(res.headers[QuickPulseConfig.endpointRedirect].toString()).host : null; | |||
There was a problem hiding this comment.
Is this the best approach? What we do here is take the host part of whatever URL is in the header, and then append QuickPulseService.svc to it. Isn't it better to just use whatever is in the header? That way if we need to change the QuickPulseService.svc part in the header - we can do that, and the SDK will follow it. Why have it hardcoded here?..
There was a problem hiding this comment.
What you are describing was the original issue we had here.
dotNET -> URI - host extraction, thus it does not care of there is QuickPulseService.svc in the call
Java -> Takes as is and uses it, thus it requires our QuickPulseService.svc be present
node -> Takes the value, DOES NOT do the host extraction and appends QuickPulseService.svc to it. hence it fails if the suffix QuickPulseService.svc is provided
What I am doing here is changing it to have the same approach as dot net. Also for the regular QP endpoint, all SDKs do the host extraction and so they do not care for the presence of QuickPulseService.svc.
So this change will make everything work the same.
There was a problem hiding this comment.
So every SDK has /QuickPulseService.svc hardcoded even for normal non-live endpoint?.. That is pretty weird, but I guess we have to follow that here as well then...
There was a problem hiding this comment.
I dont think those are for non live endpoints - and its every SDK except Java right now, I will reconcile that as well with dotnet
No description provided.