Skip to content

Commit 255d966

Browse files
committed
Address Copilot PR feedback: BrokerRequest Javadoc + result adapter exception handling
- BrokerRequest.mOnboardingSeedJson: clarify direction is client -> broker only; the broker returns the populated blob via BrokerResult.getOnboardingBlob(), not by mutating this seed field. - MsalBrokerResultAdapter.getAcquireTokenResultFromResultBundle: catch ClientException specifically (the only declared exception from brokerResultFromBundle) instead of swallowing all Exception, log at warn level so IPC/regression issues remain diagnosable, and remove the redundant null check (brokerResultFromBundle is non-null or throws). Blob contents are not logged (may carry sessionCorrelationId).
1 parent 5d7afb7 commit 255d966

2 files changed

Lines changed: 14 additions & 6 deletions

File tree

common/src/main/java/com/microsoft/identity/common/internal/broker/BrokerRequest.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,12 @@ private static final class SerializedNames {
299299

300300
/**
301301
* Onboarding telemetry seed JSON blob.
302-
* Contains sessionCorrelationId, onboarding_mode, schema_version.
303-
* Broker populates this with blocking errors and returns in the result.
302+
* Direction: client → broker (input only). Contains sessionCorrelationId,
303+
* onboarding_mode, and schema_version, supplied by the client (OneAuth/MSAL)
304+
* so the broker can construct an OnboardingTelemetryRecorder using the same
305+
* correlation id. The broker returns the populated blob (with steps and
306+
* blocking errors) via {@link BrokerResult#getOnboardingBlob()}, not via
307+
* this field.
304308
*/
305309
@Nullable
306310
@SerializedName(SerializedNames.ONBOARDING_SEED_JSON)

common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,7 @@ public AcquireTokenResult getDeviceCodeFlowTokenResultFromResultBundle(@NonNull
10151015

10161016
public @NonNull
10171017
AcquireTokenResult getAcquireTokenResultFromResultBundle(@NonNull final Bundle resultBundle) throws BaseException {
1018+
final String methodTag = TAG + ":getAcquireTokenResultFromResultBundle";
10181019
final MsalBrokerResultAdapter resultAdapter = new MsalBrokerResultAdapter();
10191020
if (resultBundle.getBoolean(AuthenticationConstants.Broker.BROKER_REQUEST_V2_SUCCESS)) {
10201021
final AcquireTokenResult acquireTokenResult = new AcquireTokenResult();
@@ -1039,14 +1040,17 @@ AcquireTokenResult getAcquireTokenResultFromResultBundle(@NonNull final Bundle r
10391040
);
10401041
}
10411042

1042-
// Extract onboarding blob from BrokerResult if present
1043+
// Extract onboarding blob from BrokerResult if present. Best-effort — telemetry
1044+
// failures must never fail an otherwise-successful auth result. Logged at warn so
1045+
// diagnosing IPC/regression issues remains possible. Blob contents are not logged
1046+
// (may contain sessionCorrelationId).
10431047
try {
10441048
final BrokerResult brokerResult = resultAdapter.brokerResultFromBundle(resultBundle);
1045-
if (brokerResult != null && brokerResult.getOnboardingBlob() != null) {
1049+
if (brokerResult.getOnboardingBlob() != null) {
10461050
acquireTokenResult.setOnboardingBlob(brokerResult.getOnboardingBlob());
10471051
}
1048-
} catch (final Exception e) {
1049-
// Best-effort — don't fail the result for telemetry
1052+
} catch (final ClientException e) {
1053+
Logger.warn(methodTag, "Failed to extract onboarding blob from broker result: " + e.getErrorCode());
10501054
}
10511055

10521056
return acquireTokenResult;

0 commit comments

Comments
 (0)