Skip to content

Commit

Permalink
fix: Pass string instead of error object to logger in session.close. (
Browse files Browse the repository at this point in the history
  • Loading branch information
hampuslavin authored Oct 24, 2024
1 parent aefba28 commit 732ccfb
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 6 deletions.
1 change: 1 addition & 0 deletions packages/serverpod/lib/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ export 'src/server/run_mode.dart';
export 'src/server/server.dart';
export 'src/server/serverpod.dart' hide ServerpodInternalMethods;
export 'src/server/session.dart' hide SessionInternalMethods;
export 'src/server/command_line_args.dart' show ServerpodLoggingMode;
2 changes: 1 addition & 1 deletion packages/serverpod/lib/src/server/session.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ abstract class Session implements DatabaseAccessor {

try {
if (_logManager == null && error != null) {
serverpod.logVerbose(error);
serverpod.logVerbose(error.toString());
if (stackTrace != null) {
serverpod.logVerbose(stackTrace.toString());
}
Expand Down
18 changes: 13 additions & 5 deletions packages/serverpod_test/lib/src/test_serverpod.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,17 @@ class InternalServerpodSession extends Session {
}
}

List<String> _getServerpodStartUpArgs(String? runMode, bool? applyMigrations) =>
List<String> _getServerpodStartUpArgs({
String? runMode,
bool? applyMigrations,
ServerpodLoggingMode? loggingMode,
}) =>
[
'-m',
runMode ?? ServerpodRunMode.test,
if (applyMigrations ?? true) '--apply-migrations',
'--logging',
loggingMode?.name ?? ServerpodLoggingMode.normal.name,
];

/// A facade for the real Serverpod instance.
Expand All @@ -95,12 +101,13 @@ class TestServerpod<T extends InternalTestEndpoints> {

/// Creates a new test serverpod instance.
TestServerpod({
bool? applyMigrations,
required bool? applyMigrations,
required EndpointDispatch endpoints,
required SerializationManagerServer serializationManager,
required this.isDatabaseEnabled,
required this.testEndpoints,
String? runMode,
required ServerpodLoggingMode? serverpodLoggingMode,
required String? runMode,
}) {
// Ignore output from the Serverpod constructor to avoid spamming the console.
// Should be changed when a proper logger is implemented.
Expand All @@ -109,8 +116,9 @@ class TestServerpod<T extends InternalTestEndpoints> {
() {
_serverpod = Serverpod(
_getServerpodStartUpArgs(
runMode,
applyMigrations,
runMode: runMode,
applyMigrations: applyMigrations,
loggingMode: serverpodLoggingMode,
),
serializationManager,
endpoints,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ void withServerpod(
_i1.TestClosure<TestEndpoints> testClosure, {
String? runMode,
bool? enableSessionLogging,
_i2.ServerpodLoggingMode? serverpodLoggingMode,
List<String>? testGroupTagsOverride,
_i1.RollbackDatabase? rollbackDatabase,
bool? applyMigrations,
Expand All @@ -39,6 +40,7 @@ void withServerpod(
runMode: runMode,
applyMigrations: applyMigrations,
isDatabaseEnabled: true,
serverpodLoggingMode: serverpodLoggingMode,
),
maybeRollbackDatabase: rollbackDatabase,
maybeEnableSessionLogging: enableSessionLogging,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import 'dart:io';

import 'package:serverpod/serverpod.dart';
import 'package:serverpod_test_server/test_util/mock_stdout.dart';
import 'package:test/test.dart';

import '../test_tools/serverpod_test_tools.dart';

class TestError extends Error {
@override
String toString() {
return 'TestError';
}
}

void main() {
withServerpod(
'Given an existing session with session logging disabled',
enableSessionLogging: false,
serverpodLoggingMode: ServerpodLoggingMode.verbose,
(sessionBuilder, endpoints) {
late MockStdout record;

setUp(() async {
record = MockStdout();
});

test(
'when closing session with error, the error should be logged and the session should be closed',
() async {
await IOOverrides.runZoned(
() async {
await sessionBuilder.build().close(error: TestError());
},
stdout: () => record,
);

expect(record.output, contains('TestError'));
});

test(
'when closing session with error and stackTrace, the error should be logged and the session should be closed',
() async {
await IOOverrides.runZoned(
() async {
await sessionBuilder.build().close(
error: TestError(),
stackTrace: StackTrace.fromString('TestStackTrace'));
},
stdout: () => record,
);

expect(record.output, contains('TestError'));
expect(record.output, contains('TestStackTrace'));
});
},
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ void withServerpod(
_i1.TestClosure<TestEndpoints> testClosure, {
String? runMode,
bool? enableSessionLogging,
_i2.ServerpodLoggingMode? serverpodLoggingMode,
List<String>? testGroupTagsOverride,
_i1.RollbackDatabase? rollbackDatabase,
bool? applyMigrations,
Expand All @@ -63,6 +64,7 @@ void withServerpod(
runMode: runMode,
applyMigrations: applyMigrations,
isDatabaseEnabled: true,
serverpodLoggingMode: serverpodLoggingMode,
),
maybeRollbackDatabase: rollbackDatabase,
maybeEnableSessionLogging: enableSessionLogging,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,10 @@ class ServerTestToolsGenerator {
..name = 'enableSessionLogging'
..named = true
..type = refer('bool?')),
Parameter((p) => p
..name = 'serverpodLoggingMode'
..named = true
..type = refer('ServerpodLoggingMode?', serverpodUrl(true))),
Parameter((p) => p
..name = 'testGroupTagsOverride'
..named = true
Expand Down Expand Up @@ -439,6 +443,7 @@ class ServerTestToolsGenerator {
'isDatabaseEnabled': literalBool(
config.isFeatureEnabled(ServerpodFeature.database),
),
'serverpodLoggingMode': refer('serverpodLoggingMode'),
},
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ void main() {
r' _i\d\.TestClosure<TestEndpoints> testClosure, \{\n'
r' String\? runMode,\n'
r' bool\? enableSessionLogging,\n'
r' _i\d\.ServerpodLoggingMode\? serverpodLoggingMode,\n'
r' List<String>\? testGroupTagsOverride,\n'
r' _i\d\.RollbackDatabase\? rollbackDatabase,\n'
r' bool\? applyMigrations,\n'
Expand Down Expand Up @@ -143,6 +144,7 @@ void main() {
r' _i\d\.TestClosure<TestEndpoints> testClosure, \{\n'
r' String\? runMode,\n'
r' bool\? enableSessionLogging,\n'
r' _i\d\.ServerpodLoggingMode\? serverpodLoggingMode,\n'
r' List<String>\? testGroupTagsOverride,\n'
r'\}\)',
));
Expand Down Expand Up @@ -188,6 +190,7 @@ void main() {
r' _i\d\.TestClosure<TestEndpoints> testClosure, \{\n'
r' String\? runMode,\n'
r' bool\? enableSessionLogging,\n'
r' _i\d\.ServerpodLoggingMode\? serverpodLoggingMode,\n'
r' List<String>\? testGroupTagsOverride,\n'
r' _i\d\.RollbackDatabase\? rollbackDatabase,\n'
r' bool\? applyMigrations,\n'
Expand Down Expand Up @@ -268,6 +271,7 @@ void main() {
r' _i\d\.TestClosure<TestEndpoints> testClosure, \{\n'
r' String\? runMode,\n'
r' bool\? enableSessionLogging,\n'
r' _i\d\.ServerpodLoggingMode\? serverpodLoggingMode,\n'
r' List<String>\? testGroupTagsOverride,\n'
r' _i\d\.RollbackDatabase\? rollbackDatabase,\n'
r' bool\? applyMigrations,\n'
Expand Down

0 comments on commit 732ccfb

Please sign in to comment.