Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: retire error-message #11190

Merged
merged 4 commits into from
Mar 27, 2024
Merged

fix: retire error-message #11190

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e36a032
fix: retire error-message
jayzhang Mar 26, 2024
69d25b5
test: ut
jayzhang Mar 26, 2024
971cfc3
test: ut
jayzhang Mar 26, 2024
576c1ac
test: ut
jayzhang Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/fx-core/src/common/telemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ export function fillInTelemetryPropsForFxError(
props[TelemetryConstants.properties.errorCode] =
props[TelemetryConstants.properties.errorCode] || errorCode;
props[TelemetryConstants.properties.errorType] = errorType;
props[TelemetryConstants.properties.errorMessage] = error.message;
props[TelemetryConstants.properties.errorStack] = error.stack !== undefined ? error.stack : ""; // error stack will not append in error-message any more
// props[TelemetryConstants.properties.errorMessage] = error.message; // error-message is retired
// props[TelemetryConstants.properties.errorStack] = error.stack !== undefined ? error.stack : ""; // error stack will not append in error-message any more
props[TelemetryConstants.properties.errorName] = error.name;

// append global context properties
Expand All @@ -250,12 +250,12 @@ export function fillInTelemetryPropsForFxError(
props[TelemetryConstants.properties.errorInnerCode] = error.innerError["code"];
}

if (error.innerError) {
props[TelemetryConstants.properties.innerError] = JSON.stringify(
error.innerError,
Object.getOwnPropertyNames(error.innerError)
);
}
// if (error.innerError) { // inner-error is retired
// props[TelemetryConstants.properties.innerError] = JSON.stringify(
// error.innerError,
// Object.getOwnPropertyNames(error.innerError)
// );
// }

if (error.categories) {
props[TelemetryConstants.properties.errorCat] = error.categories.join("|");
Expand Down
4 changes: 2 additions & 2 deletions packages/fx-core/src/error/script.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class ScriptTimeoutError extends UserError {
const errorOptions: UserErrorOptions = {
source: "script",
name: "ScriptTimeoutError",
message: getDefaultString(key, cmd),
message: getDefaultString(key, ""), //the script content maybe contains securit data that should not be reported in telemetry
displayMessage: getLocalizedString(key, cmd),
error: error,
categories: [ErrorCategory.External],
Expand All @@ -32,7 +32,7 @@ export class ScriptExecutionError extends UserError {
const errorOptions: UserErrorOptions = {
source: "script",
name: "ScriptExecutionError",
message: getDefaultString(key, script, message),
message: getDefaultString(key, "", message), //the script content maybe contains securit data that should not be reported in telemetry
displayMessage: getLocalizedString(key, script, message),
error: error,
categories: [ErrorCategory.External],
Expand Down
6 changes: 3 additions & 3 deletions packages/fx-core/tests/component/driver/aad/create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,9 @@ describe("aadAppCreate", async () => {
expect(endTelemetry.properties.success).to.equal("no");
expect(endTelemetry.properties["error-code"]).to.equal("aadAppCreate.HttpClientError");
expect(endTelemetry.properties["error-type"]).to.equal("user");
expect(endTelemetry.properties["error-message"]).to.equal(
'A http client error happened while performing the aadApp/create task. The error response is: {"error":{"code":"Request_BadRequest","message":"Invalid value specified for property \'displayName\' of resource \'Application\'."}}'
);
// expect(endTelemetry.properties["error-message"]).to.equal(
Copy link
Contributor

Choose a reason for hiding this comment

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

will there be another property to store error message?

Copy link
Member Author

Choose a reason for hiding this comment

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

We will use another property name after we can make sure that the error message is in our total control. Retire this for now.

// 'A http client error happened while performing the aadApp/create task. The error response is: {"error":{"code":"Request_BadRequest","message":"Invalid value specified for property \'displayName\' of resource \'Application\'."}}'
// );
});

it("should send telemetries with error stack", async () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/fx-core/tests/component/driver/aad/update.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,9 @@ describe("aadAppUpdate", async () => {
expect(endTelemetry.properties.success).to.equal("no");
expect(endTelemetry.properties["error-code"]).to.equal("aadAppUpdate.HttpServerError");
expect(endTelemetry.properties["error-type"]).to.equal("system");
expect(endTelemetry.properties["error-message"]).to.equal(
'A http server error happened while performing the aadApp/update task. Please try again later. The error response is: {"error":{"code":"InternalServerError","message":"Internal server error"}}'
);
// expect(endTelemetry.properties["error-message"]).to.equal(
// 'A http server error happened while performing the aadApp/update task. Please try again later. The error response is: {"error":{"code":"InternalServerError","message":"Internal server error"}}'
// );
});

it("should throw error when missing required environment variable in manifest", async () => {
Expand Down
3 changes: 0 additions & 3 deletions packages/fx-core/tests/component/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ describe("TeamsFxTelemetryReporter", () => {
success: "no",
"error-code": "source.name",
"error-type": "user",
"error-message": "message",
});
reporterCalled = true;
});
Expand All @@ -299,7 +298,6 @@ describe("TeamsFxTelemetryReporter", () => {
success: "no",
"error-code": "my error code",
"error-type": "user",
"error-message": "message",
"my-property": "value",
});
reporterCalled = true;
Expand All @@ -322,7 +320,6 @@ describe("TeamsFxTelemetryReporter", () => {
.stub(mockedTelemetryReporter, "sendTelemetryErrorEvent")
.callsFake((eventName, properties, measurements, errorProps) => {
expect(errorProps).include("test");
expect(errorProps).include("error-message");
reporterCalled = true;
});

Expand Down
Loading