Skip to content

Centralised logging. #26

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

Merged
microbit-matt-hillsdon merged 1 commit into from
Mar 15, 2021
Merged

Centralised logging. #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 57 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@emotion/react": "^11.1.5",
"@emotion/styled": "^11.1.5",
"@microbit/microbit-fs": "^0.9.1",
"@sentry/browser": "^6.2.2",
"@testing-library/jest-dom": "^5.11.9",
"@testing-library/react": "^11.2.5",
"@testing-library/user-event": "^12.8.1",
Expand Down
20 changes: 20 additions & 0 deletions public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@
name="twitter:description"
content="A Python Editor for the BBC micro:bit, built by the Micro:bit Educational Foundation and the global Python Community."
/>
<script
async
src="https://www.googletagmanager.com/gtag/js?id=%REACT_APP_GA_MEASUREMENT_ID%"
></script>
<script>
var stage = "%REACT_APP_STAGE%";
if (stage === "PRODUCTION") {
window.dataLayer = window.dataLayer || [];
window.gaMeasurementId = "%REACT_APP_GA_MEASUREMENT_ID%";
window.gtag = function () {
window.dataLayer.push(arguments);
};
gtag("js", new Date());
var options = {
anonymize_ip: true,
cookie_prefix: "%REACT_APP_GA_COOKIE_PREFIX%",
};
gtag("config", window.gaMeasurementId, options);
}
</script>
</head>
<body>
<noscript
Expand Down
25 changes: 15 additions & 10 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ import {
import { useLocalStorage } from "./common/use-local-storage";
import Project from "./project/Project";
import ProjectDropTarget from "./project/ProjectDropTarget";
import { LoggingContext } from "./logging/logging-hooks";
import { DefaultLogging } from "./logging/default";

const device = new MicrobitWebUSBConnection();
const logging = new DefaultLogging();
const device = new MicrobitWebUSBConnection({ logging });
const fs = new FileSystem();

const App = () => {
Expand All @@ -35,15 +38,17 @@ const App = () => {

return (
<ChakraProvider theme={theme}>
<DeviceContext.Provider value={device}>
<FileSystemContext.Provider value={fs}>
<SettingsContext.Provider value={settings}>
<ProjectDropTarget>
<Project />
</ProjectDropTarget>
</SettingsContext.Provider>
</FileSystemContext.Provider>
</DeviceContext.Provider>
<LoggingContext.Provider value={logging}>
<DeviceContext.Provider value={device}>
<FileSystemContext.Provider value={fs}>
<SettingsContext.Provider value={settings}>
<ProjectDropTarget>
<Project />
</ProjectDropTarget>
</SettingsContext.Provider>
</FileSystemContext.Provider>
</DeviceContext.Provider>
</LoggingContext.Provider>
</ChakraProvider>
);
};
Expand Down
21 changes: 9 additions & 12 deletions src/common/use-action-feedback.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@ import { ReactNode } from "react";
import { Link, useToast } from "@chakra-ui/react";
import { useMemo } from "react";
import config from "../config";
import { useLogging } from "../logging/logging-hooks";
import { Logging } from "../logging/logging";

export class ActionFeedback {
constructor(private toast: ReturnType<typeof useToast>) {}
constructor(
private toast: ReturnType<typeof useToast>,
private logging: Logging
) {}

/**
* Handles an error.
Expand All @@ -18,10 +23,6 @@ export class ActionFeedback {
description: ReactNode;
error?: any;
}) {
// For now at least.
if (error) {
console.error(error);
}
this.toast({
title,
status: "error",
Expand All @@ -43,10 +44,6 @@ export class ActionFeedback {
description: ReactNode;
error?: any;
}) {
// For now at least.
if (error) {
console.error(error);
}
this.toast({
title,
status: "warning",
Expand Down Expand Up @@ -75,8 +72,7 @@ export class ActionFeedback {
* @param error the error thrown.
*/
unexpectedError(error: Error) {
// For now at least.
console.error(error);
this.logging.error(error);
this.toast({
title: "An unexpected error occurred",
status: "error",
Expand Down Expand Up @@ -105,7 +101,8 @@ export class ActionFeedback {
*/
const useActionFeedback = () => {
const toast = useToast();
return useMemo(() => new ActionFeedback(toast), [toast]);
const logging = useLogging();
return useMemo(() => new ActionFeedback(toast, logging), [toast, logging]);
};

export default useActionFeedback;
6 changes: 3 additions & 3 deletions src/device/dap-wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { CortexM, DAPLink, WebUSB } from "dapjs";
import { Logging } from "../logging/logging";
import {
ApReg,
CortexSpecialReg,
Expand All @@ -7,7 +8,6 @@ import {
DapVal,
FICR,
} from "./constants";
import { log } from "./logging";
import {
apReg,
bufferConcat,
Expand All @@ -24,7 +24,7 @@ export class DAPWrapper {
_numPages: number | undefined;
private initialConnectionComplete: boolean = false;

constructor(public device: USBDevice) {
constructor(public device: USBDevice, private logging: Logging) {
this.transport = new WebUSB(this.device);
this.daplink = new DAPLink(this.transport);
this.cortexM = new CortexM(this.transport);
Expand Down Expand Up @@ -232,7 +232,7 @@ export class DAPWrapper {
} catch (e) {
if (e.dapWait) {
// Retry after a delay if required.
log(`Transfer wait, write block`);
this.logging.log(`Transfer wait, write block`);
await new Promise((resolve) => setTimeout(resolve, 100));
return await this.writeBlockCore(addr, words);
} else {
Expand Down
5 changes: 4 additions & 1 deletion src/device/device.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
MicrobitWebUSBConnection,
} from "./device";
import { USB } from "webusb";
import { NullLogging } from "../logging/null";

const describeDeviceOnly = process.env.TEST_MODE_DEVICE
? describe
Expand All @@ -19,7 +20,9 @@ const describeDeviceOnly = process.env.TEST_MODE_DEVICE
describe("MicrobitWebUSBConnection (WebUSB unsupported)", () => {
it("notices if WebUSB isn't supported", () => {
(global as any).navigator = {};
const microbit = new MicrobitWebUSBConnection();
const microbit = new MicrobitWebUSBConnection({
logging: new NullLogging(),
});
expect(microbit.status).toBe(ConnectionStatus.NOT_SUPPORTED);
});
});
Expand Down
45 changes: 30 additions & 15 deletions src/device/device.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import EventEmitter from "events";
import { FlashDataSource } from "../fs/fs";
import { Logging } from "../logging/logging";
import translation from "../translation";
import { BoardId } from "./board-id";
import { DAPWrapper } from "./dap-wrapper";
import { log } from "./logging";
import { PartialFlashing } from "./partial-flashing";

/**
Expand Down Expand Up @@ -58,6 +58,13 @@ export class WebUSBError extends Error {
}
}

interface MicrobitWebUSBConnectionOptions {
// We should copy this type when extracting a library, and make it optional.
// Coupling for now to make it easy to evolve.

logging: Logging;
}

/**
* Tracks WebUSB connection status.
*/
Expand Down Expand Up @@ -110,6 +117,17 @@ export class MicrobitWebUSBConnection extends EventEmitter {
this.emit(EVENT_SERIAL_DATA, data);
};

private logging: Logging;

constructor(options: MicrobitWebUSBConnectionOptions) {
super();
this.logging = options.logging;
}

private log(v: any) {
this.logging.log(v);
}

async initialize(): Promise<void> {
if (navigator.usb) {
navigator.usb.addEventListener("disconnect", this.handleDisconnect);
Expand Down Expand Up @@ -169,9 +187,9 @@ export class MicrobitWebUSBConnection extends EventEmitter {
if (!this.connection) {
await this.connectInternal(false);
} else {
log("Stopping serial before flash");
this.log("Stopping serial before flash");
this.stopSerial();
log("Reconnecting before flash");
this.log("Reconnecting before flash");
await this.connection.reconnectAsync();
}
if (!this.connection) {
Expand All @@ -184,22 +202,22 @@ export class MicrobitWebUSBConnection extends EventEmitter {
const boardIdString = this.connection.boardId;
const boardId = BoardId.parse(boardIdString);
const data = await dataSource(boardId);
const flashing = new PartialFlashing(this.connection);
const flashing = new PartialFlashing(this.connection, this.logging);
try {
if (partial) {
await flashing.flashAsync(data.bytes, data.intelHex, progress);
} else {
await flashing.fullFlashAsync(data.intelHex, progress);
}

log("Reinstating serial after flash");
this.log("Reinstating serial after flash");
await this.connection.daplink.connect();

// This is async but won't return until we've finished serial.
// TODO: consider error handling here, via an event?
this.connection
.startSerial(this.serialListener)
.then(() => log("Finished listening for serial data"))
.then(() => this.log("Finished listening for serial data"))
.catch((e) => this.emit(EVENT_SERIAL_ERROR, e));
} finally {
progress(undefined);
Expand All @@ -216,7 +234,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {
this.connection.disconnectAsync();
}
} catch (e) {
log("Error during disconnection:\r\n" + e);
this.log("Error during disconnection:\r\n" + e);
} finally {
this.connection = undefined;
this.setStatus(ConnectionStatus.NOT_CONNECTED);
Expand All @@ -225,7 +243,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {

private setStatus(newStatus: ConnectionStatus) {
this.status = newStatus;
log("Device status " + newStatus);
this.log("Device status " + newStatus);
this.emit(EVENT_STATUS, this.status);
}

Expand All @@ -234,11 +252,11 @@ export class MicrobitWebUSBConnection extends EventEmitter {
return await f();
} catch (e) {
// Log error to console for feedback
log("An error occurred whilst attempting to use WebUSB.");
log(
this.log("An error occurred whilst attempting to use WebUSB.");
this.log(
"Details of the error can be found below, and may be useful when trying to replicate and debug the error."
);
log(e);
this.log(e);

// Disconnect from the microbit.
// Any new connection reallocates all the internals.
Expand Down Expand Up @@ -268,7 +286,7 @@ export class MicrobitWebUSBConnection extends EventEmitter {
private async connectInternal(serial: boolean): Promise<void> {
if (!this.connection) {
const device = await this.chooseDevice();
this.connection = new DAPWrapper(device);
this.connection = new DAPWrapper(device, this.logging);
}
await this.connection.reconnectAsync();
if (serial) {
Expand Down Expand Up @@ -303,7 +321,6 @@ const enrichedError = (err: any): WebUSBError => {
}
switch (typeof err) {
case "object":
log("Caught in Promise or Error object");
// We might get Error objects as Promise rejection arguments
if (!err.message && err.promise && err.reason) {
err = err.reason;
Expand Down Expand Up @@ -339,11 +356,9 @@ const enrichedError = (err: any): WebUSBError => {
}
case "string": {
// Caught a string. Example case: "Flash error" from DAPjs
log("Caught a string");
return genericErrorSuggestingReconnect(err);
}
default: {
log("Unexpected error type: " + typeof err);
return genericErrorSuggestingReconnect(err);
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/device/logging.ts
View file
Open in desktop

This file was deleted.

Loading