From 61e0931eb5dacd54fe35003bd6a2aa5892fabb44 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 19 Sep 2023 15:13:12 +0200 Subject: [PATCH 1/3] Fix error logging With the upgrade to Webpack 5, we accidentally broke the logger factory because the `process.env.VUE_APP_LOGGER` was no longer defined. We now define it in the webpack environment configuration files. Additionally, we only override the error logger in production, to send errors to our error logging tool. --- .env.development | 1 - .env.production | 3 --- src/createVueApp.ts | 14 ++++++++------ src/logger.ts | 9 ++++++++- webpack/env/dev.env.js | 1 + webpack/env/prod.env.js | 3 +++ webpack/webpack.config.common.js | 4 ---- 7 files changed, 20 insertions(+), 15 deletions(-) delete mode 100644 .env.development delete mode 100644 .env.production diff --git a/.env.development b/.env.development deleted file mode 100644 index 9859eb8d2..000000000 --- a/.env.development +++ /dev/null @@ -1 +0,0 @@ -VUE_APP_LOGGER=console \ No newline at end of file diff --git a/.env.production b/.env.production deleted file mode 100644 index 865f92e97..000000000 --- a/.env.production +++ /dev/null @@ -1,3 +0,0 @@ -VUE_APP_LOGGER=errbit -VUE_APP_ERRBIT_HOST=https://logging.wikimedia.de -VUE_APP_ERRBIT_PROJECT_KEY=6dfc2d772bf50af73ff529af47b20614 \ No newline at end of file diff --git a/src/createVueApp.ts b/src/createVueApp.ts index 89c9a1e9d..f06001f27 100644 --- a/src/createVueApp.ts +++ b/src/createVueApp.ts @@ -12,12 +12,14 @@ export function createVueApp( rootComponent: Component, messages: Messages, root } ); app.use( i18n ); - app.config.errorHandler = ( err, vm, info ) => { - createLogger().notify( { - error: err, - params: { info: info }, - } ); - }; + if ( process.env.NODE_ENV === 'production' ) { + app.config.errorHandler = ( err, vm, info ) => { + createLogger().notify( { + error: err, + params: { info: info }, + } ); + }; + } return app; } diff --git a/src/logger.ts b/src/logger.ts index 578045a8d..9880bb966 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -35,7 +35,14 @@ class ConsoleLogger implements Logger { } } -// TODO: See if there's an alternative to using process.env here +/** + * Factory function to create a logger based on the VUE_APP_LOGGER environment variable. + * The variable can be set to 'errbit' or 'console'. + * + * When using Webpack >=5, the variable needs to be defined in webpack configuration + * using the DefinePlugin ('process.env.VUE_APP_LOGGER': JSON.stringify( 'errbit' )) + * or the EnvironmentPlugin ({'VUE_APP_LOGGER': 'errbit'}). + */ export default function createLogger(): Logger { switch ( process.env.VUE_APP_LOGGER ) { case LOGGER_CONSOLE: diff --git a/webpack/env/dev.env.js b/webpack/env/dev.env.js index 7c2fb134b..ddcbe528b 100644 --- a/webpack/env/dev.env.js +++ b/webpack/env/dev.env.js @@ -1,3 +1,4 @@ module.exports = { NODE_ENV: 'development', + VUE_APP_LOGGER: 'console', }; diff --git a/webpack/env/prod.env.js b/webpack/env/prod.env.js index 6ca0d3265..2fae3cead 100644 --- a/webpack/env/prod.env.js +++ b/webpack/env/prod.env.js @@ -1,3 +1,6 @@ module.exports = { NODE_ENV: 'production', + VUE_APP_LOGGER: 'errbit', + VUE_APP_ERRBIT_HOST: 'https://logging.wikimedia.de', + VUE_APP_ERRBIT_PROJECT_KEY: '6dfc2d772bf50af73ff529af47b20614', }; diff --git a/webpack/webpack.config.common.js b/webpack/webpack.config.common.js index 93e1f5057..163459e5a 100644 --- a/webpack/webpack.config.common.js +++ b/webpack/webpack.config.common.js @@ -1,5 +1,4 @@ 'use strict'; -const webpack = require( 'webpack' ); const ForkTsCheckerWebpackPlugin = require( 'fork-ts-checker-webpack-plugin' ); const { VueLoaderPlugin } = require( 'vue-loader' ); const MiniCSSExtractPlugin = require( 'mini-css-extract-plugin' ); @@ -115,9 +114,6 @@ const webpackConfig = { }, }, } ), - new webpack.ProvidePlugin( { - process: 'process/browser', - } ), ], }; From 4257847a420888d7e8b82b99be95d3a8ecaff022 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 19 Sep 2023 18:45:20 +0200 Subject: [PATCH 2/3] Make errbit key a build-time secret --- .drone.yml | 3 +++ README.md | 7 +++++++ webpack/env/prod.env.js | 6 ++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.drone.yml b/.drone.yml index e3a6583b6..dc76ab738 100644 --- a/.drone.yml +++ b/.drone.yml @@ -8,6 +8,9 @@ steps: volumes: - name: build-dir path: /build + environment: + VUE_APP_ERRBIT_PROJECT_KEY: + from_secret: errbit_key commands: - npm install - npm run build diff --git a/README.md b/README.md index 18657e1bc..291fd846e 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,13 @@ them into the directory `web/skins/laika` in the Fundraising Application directory. You can ignore or delete the generated HTML files, they are an unused byproduct of the build process. +By default, the production build will send errors to our error collection +service. When building for production, you must provide the API key for +the service in the environment variable `VUE_APP_ERRBIT_PROJECT_KEY`. Our +CI/CD system does this automatically, if you're building locally, you can +run the command like this: + + VUE_APP_ERRBIT_PROJECT_KEY= npm run build ## Where to put images and fonts and how to reference them Put all images, fonts and other non-bundled resources into subdirectories diff --git a/webpack/env/prod.env.js b/webpack/env/prod.env.js index 2fae3cead..5bbf69a4c 100644 --- a/webpack/env/prod.env.js +++ b/webpack/env/prod.env.js @@ -1,6 +1,8 @@ module.exports = { NODE_ENV: 'production', VUE_APP_LOGGER: 'errbit', - VUE_APP_ERRBIT_HOST: 'https://logging.wikimedia.de', - VUE_APP_ERRBIT_PROJECT_KEY: '6dfc2d772bf50af73ff529af47b20614', + VUE_APP_ERRBIT_HOST: process.env.VUE_APP_ERRBIT_HOST || 'https://logging.wikimedia.de', + // should be set in the CI/CD pipeline or when running locally + // e.g. "VUE_APP_ERRBIT_PROJECT_KEY=1234567890abcdef npm run build" + VUE_APP_ERRBIT_PROJECT_KEY: process.env.VUE_APP_ERRBIT_PROJECT_KEY || '', }; From 342aba897c157ae48a8055c264f956d7f2038c61 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Thu, 21 Sep 2023 13:18:34 +0200 Subject: [PATCH 3/3] Remove ConsoleLogger We currently don't use it and can save a few bytes by removing it. --- src/logger.ts | 15 +++------------ webpack/env/dev.env.js | 1 - 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 9880bb966..24946dd8d 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -1,7 +1,6 @@ import { Notifier } from '@airbrake/browser'; const LOGGER_ERRBIT: string = 'errbit'; -const LOGGER_CONSOLE: string = 'console'; interface Logger { notify( error: object ): void @@ -28,25 +27,17 @@ class SilentLogger implements Logger { notify( error: object ) {} /* eslint-disable-line @typescript-eslint/no-unused-vars */ } -class ConsoleLogger implements Logger { - notify( error: object ) { - // eslint-disable-next-line - console.log( error ); - } -} - /** * Factory function to create a logger based on the VUE_APP_LOGGER environment variable. - * The variable can be set to 'errbit' or 'console'. + * The only supported value currently is 'errbit'. In the future we might re-introduce + * a 'console' logger (see Git commit history of this file). * * When using Webpack >=5, the variable needs to be defined in webpack configuration * using the DefinePlugin ('process.env.VUE_APP_LOGGER': JSON.stringify( 'errbit' )) * or the EnvironmentPlugin ({'VUE_APP_LOGGER': 'errbit'}). */ export default function createLogger(): Logger { - switch ( process.env.VUE_APP_LOGGER ) { - case LOGGER_CONSOLE: - return new ConsoleLogger(); + switch ( process?.env?.VUE_APP_LOGGER ) { case LOGGER_ERRBIT: return new ErrbitLogger( process.env.VUE_APP_ERRBIT_HOST || '', diff --git a/webpack/env/dev.env.js b/webpack/env/dev.env.js index ddcbe528b..7c2fb134b 100644 --- a/webpack/env/dev.env.js +++ b/webpack/env/dev.env.js @@ -1,4 +1,3 @@ module.exports = { NODE_ENV: 'development', - VUE_APP_LOGGER: 'console', };