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.

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

POC: Move package:web access to separate package #2110

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ build/
.cxx/
.vscode/


.test_coverage.dart
dart/coverage/*
logging/coverage/*
Expand All @@ -25,6 +24,7 @@ sqflite/coverage/*
drift/coverage/*
hive/coverage/*
isar/coverage/*
web/coverage/*

pubspec.lock
Podfile.lock
Expand Down
2 changes: 2 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ export 'src/sentry_span_operations.dart';
export 'src/utils.dart';
// spotlight debugging
export 'src/spotlight.dart';
// Window wrappers
export 'src/web/window.dart';
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import '../../event_processor.dart';
import '../../sentry_options.dart';
import 'io_enricher_event_processor.dart'
if (dart.library.html) 'web_enricher_event_processor.dart';
if (dart.library.html) 'web_enricher_event_processor.dart'
if (dart.library.js_interop) 'web_enricher_event_processor.dart';

abstract class EnricherEventProcessor implements EventProcessor {
factory EnricherEventProcessor(SentryOptions options) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import 'dart:html' as html show window, Window;

import '../../../sentry.dart';
import 'enricher_event_processor.dart';

import '../../web/noop_window.dart'
if (dart.library.html) '../../web/http_window.dart'
if (dart.library.js_interop) '../../web/web_window.dart';

EnricherEventProcessor enricherEventProcessor(SentryOptions options) {
return WebEnricherEventProcessor(
html.window,
createWindow(options),
options,
);
}
Expand All @@ -16,7 +18,7 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {
this._options,
);

final html.Window _window;
final Window _window;

final SentryOptions _options;

Expand Down Expand Up @@ -60,11 +62,11 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {
memorySize: device?.memorySize ?? _getMemorySize(),
orientation: device?.orientation ?? _getScreenOrientation(),
screenHeightPixels: device?.screenHeightPixels ??
_window.screen?.available.height.toInt(),
_window.screen.availableHeight,
screenWidthPixels:
device?.screenWidthPixels ?? _window.screen?.available.width.toInt(),
device?.screenWidthPixels ?? _window.screen.availableWidth,
screenDensity:
device?.screenDensity ?? _window.devicePixelRatio.toDouble(),
device?.screenDensity ?? _window.devicePixelRatio,
);
}

Expand All @@ -77,14 +79,11 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {

SentryOrientation? _getScreenOrientation() {
// https://developer.mozilla.org/en-US/docs/Web/API/ScreenOrientation
final screenOrientation = _window.screen?.orientation;
if (screenOrientation != null) {
if (screenOrientation.type?.startsWith('portrait') ?? false) {
return SentryOrientation.portrait;
}
if (screenOrientation.type?.startsWith('landscape') ?? false) {
return SentryOrientation.landscape;
}
final screenOrientation = _window.screen.orientation;
if (screenOrientation == ScreenOrientation.portrait) {
return SentryOrientation.portrait;
} else if (screenOrientation == ScreenOrientation.landscape) {
return SentryOrientation.landscape;
}
return null;
}
Expand Down
4 changes: 2 additions & 2 deletions dart/lib/src/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,14 @@ class Sentry {
}) async {
final sentryOptions = options ?? SentryOptions();

await _initDefaultValues(sentryOptions);
_setEnvironmentVariables(sentryOptions);

try {
final config = optionsConfiguration(sentryOptions);
if (config is Future) {
await config;
}
await _initDefaultValues(sentryOptions);
Copy link
Collaborator Author

@denrase denrase Jun 18, 2024

Choose a reason for hiding this comment

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

Changing this order probably breaks stuff in unpredictable ways...

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to change the ordering?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok nvm I understand.

We'll have to think further about this. Like you said this might break stuff / have side effects

Copy link
Contributor

@buenaflor buenaflor Jun 19, 2024

Choose a reason for hiding this comment

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

We can have a static setWindow() function which sets an internal window and then assign that to options when calling init.

Window _window = NoopWindow();
void setWindow(Window window) {
  if (!PlatformChecker().isWeb) {
    return;
  }
  _window = window;
}

// and then inside init()
final sentryOptions = options ?? SentryOptions();

options?.window = _window;

The user just needs to call the setWindow function before SentryFlutter.init if they wanna do wasm

Sentry.setWindow(...)
SentryFlutter.init(...)

at least that way we can keep the ordering. maybe there's better ways

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still a breaking change, isn't it? As in, things that worked before for users won't work anymore unless they change their code.

Copy link
Contributor

@buenaflor buenaflor Jun 20, 2024

Choose a reason for hiding this comment

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

This is still a breaking change, isn't it? As in, things that worked before for users won't work anymore unless they change their code.

tbh I'm not sure, wasm compilation only works on the latest stable channel so they all have to upgrade flutter afaik and it's not working right now for them anyway.

non-wasm users won't have to care about this stuff

Copy link
Collaborator

Choose a reason for hiding this comment

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

What I meant is that it would be breaking for existing users targeting web? Or am I missing something.

} catch (exception, stackTrace) {
sentryOptions.logger(
SentryLevel.error,
Expand All @@ -73,7 +74,6 @@ class Sentry {
}

static Future<void> _initDefaultValues(SentryOptions options) async {
_setEnvironmentVariables(options);

// Throws when running on the browser
if (!options.platformChecker.isWeb) {
Expand Down
2 changes: 2 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ class SentryOptions {
/// ```
Spotlight spotlight = Spotlight(enabled: false);

Window? Function() window = () { return null; };

SentryOptions({this.dsn, PlatformChecker? checker}) {
if (checker != null) {
platformChecker = checker;
Expand Down
70 changes: 70 additions & 0 deletions dart/lib/src/web/http_window.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import 'dart:html' as html show window, Window;

import '../sentry_options.dart';
import 'window.dart';

Window createWindow(SentryOptions options) {
return HttpWindow(html.window);
}

class HttpWindow implements Window {
HttpWindow(this._window);

final html.Window _window;

@override
WindowScreen get screen => HttpWindowScreen(_window);

@override
WindowNavigator get navigator => HttpWindowNavigator(_window);

@override
WindowLocation get location => HttpWindowLocation(_window);

@override
double get devicePixelRatio => _window.devicePixelRatio.toDouble();
}

class HttpWindowScreen implements WindowScreen {
HttpWindowScreen(this._window);

final html.Window _window;

@override
int get availableHeight => _window.screen?.available.height.toInt() ?? 0;

@override
int get availableWidth => _window.screen?.available.width.toInt() ?? 0;

@override
ScreenOrientation? get orientation =>
_window.screen?.orientation?.type == "portrait"
? ScreenOrientation.portrait
: _window.screen?.orientation?.type == "landscape"
? ScreenOrientation.landscape
: null;
}

class HttpWindowNavigator implements WindowNavigator {
HttpWindowNavigator(this._window);

final html.Window _window;

@override
String get userAgent => _window.navigator.userAgent;

@override
bool? get onLine => _window.navigator.onLine;

@override
double? get deviceMemory => _window.navigator.deviceMemory?.toDouble();
}

class HttpWindowLocation implements WindowLocation {
HttpWindowLocation(this._window);

final html.Window _window;

@override
String? get pathname => _window.location.pathname;
}
55 changes: 55 additions & 0 deletions dart/lib/src/web/noop_window.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@

import '../sentry_options.dart';
import 'window.dart';

Window createWindow(SentryOptions options) {
return NoopWindow();
}

class NoopWindow implements Window {

@override
WindowScreen get screen => NoopWindowScreen();

@override
WindowNavigator get navigator => NoopWindowNavigator();

@override
WindowLocation get location => NoopWindowLocation();

@override
double get devicePixelRatio => 1.0;
}

class NoopWindowScreen implements WindowScreen {
NoopWindowScreen();

@override
int get availableHeight => 0;

@override
int get availableWidth => 0;

@override
ScreenOrientation? get orientation => null;
}

class NoopWindowNavigator implements WindowNavigator {
NoopWindowNavigator();

@override
String get userAgent => "--";

@override
bool? get onLine => null;

@override
double? get deviceMemory => null;
}

class NoopWindowLocation implements WindowLocation {
NoopWindowLocation();

@override
String? get pathname => null;
}
8 changes: 8 additions & 0 deletions dart/lib/src/web/web_window.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import '../sentry_options.dart';
import 'noop_window.dart';
import 'window.dart';

// Get window from options or noop
Window createWindow(SentryOptions options) {
return options.window() ?? NoopWindow();
Copy link
Contributor

Choose a reason for hiding this comment

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

We create the versions.dart that contains the package version and name and add it to the options like here

since we don't use the options inside the sentry_web package, maybe we can set it here?

technically the user could provide their own Window implementation but I don't think it's likely that they do that

}
36 changes: 36 additions & 0 deletions dart/lib/src/web/window.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
abstract class Window {

WindowScreen get screen;
WindowNavigator get navigator;
WindowLocation get location;

double get devicePixelRatio;
}

abstract class WindowScreen {
WindowScreen();

int get availableHeight;
int get availableWidth;

ScreenOrientation? get orientation;
}

abstract class WindowNavigator {
WindowNavigator();

String get userAgent;

bool? get onLine;

double? get deviceMemory;
}

abstract class WindowLocation {
String? get pathname;
}

enum ScreenOrientation {
portrait,
landscape,
}
18 changes: 9 additions & 9 deletions flutter/example/lib/auto_close_screen.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'package:dio/dio.dart';
// import 'package:dio/dio.dart';
import 'package:flutter/material.dart';
import 'package:sentry_dio/sentry_dio.dart';
// import 'package:sentry_dio/sentry_dio.dart';
import 'package:sentry_flutter/sentry_flutter.dart';

import 'main.dart';
Expand All @@ -25,13 +25,13 @@ class AutoCloseScreenState extends State<AutoCloseScreen> {
}

Future<void> _doComplexOperationThenClose() async {
final dio = Dio();
dio.addSentry();
try {
await dio.get<String>(exampleUrl);
} catch (exception, stackTrace) {
await Sentry.captureException(exception, stackTrace: stackTrace);
}
// final dio = Dio();
// dio.addSentry();
// try {
// await dio.get<String>(exampleUrl);
// } catch (exception, stackTrace) {
// await Sentry.captureException(exception, stackTrace: stackTrace);
// }
SentryFlutter.reportFullyDisplayed();
// ignore: use_build_context_synchronously
Navigator.of(context).pop();
Expand Down
14 changes: 7 additions & 7 deletions flutter/example/lib/drift/connection/native.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'package:drift/backends.dart';
import 'package:drift/drift.dart';
import 'package:drift/native.dart';

QueryExecutor inMemoryExecutor() {
return NativeDatabase.memory();
}
// import 'package:drift/backends.dart';
// import 'package:drift/drift.dart';
// import 'package:drift/native.dart';
//
// QueryExecutor inMemoryExecutor() {
// return NativeDatabase.memory();
// }
34 changes: 17 additions & 17 deletions flutter/example/lib/drift/connection/unsupported.dart
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import 'package:drift/drift.dart';

Never _unsupported() {
throw UnsupportedError(
'No suitable database implementation was found on this platform.');
}

// Depending on the platform the app is compiled to, the following stubs will
// be replaced with the methods in native.dart or web.dart

QueryExecutor inMemoryExecutor() {
return _unsupported();
}

Future<void> validateDatabaseSchema(GeneratedDatabase database) async {
_unsupported();
}
// import 'package:drift/drift.dart';
//
// Never _unsupported() {
// throw UnsupportedError(
// 'No suitable database implementation was found on this platform.');
// }
//
// // Depending on the platform the app is compiled to, the following stubs will
// // be replaced with the methods in native.dart or web.dart
//
// QueryExecutor inMemoryExecutor() {
// return _unsupported();
// }
//
// Future<void> validateDatabaseSchema(GeneratedDatabase database) async {
// _unsupported();
// }
Loading
Loading