nitrictech / cli

Nitric CLI. Manage and run Nitric apps locally and deploy to any cloud.
https://nitric.io
Apache License 2.0
25 stars 10 forks source link

Unknown service nitric.proto.sql.v1.Sql error during collection #812

Open tjholm opened 6 days ago

tjholm commented 6 days ago

Raising as an issue from: https://discord.com/channels/955259353043173427/955259353043173430/1300940268618383421

Example app: https://github.com/chimon2000/notes_nitric

The issue occurring in this example is that the Sql runtime API like our other runtime APIs is not registered during collection time, this is intended by design, this should be linked to produce an error during collection that highlights the issue though so this is definitely a bug.

However reading a connection string isn't really a side-effect producing operation, so it is possible to make an exception in this case, but doing so will require documentation of specific exceptions to this rule.

The example provided above is written in Dart, and being able to support this style of retrieval of the connection string in this specific case is very convenient given all global variables in Dart are lazily evaluated.

There are a few options to address this:

Option 1 - Implement the connection string SQL method during requirements gathering

Add the SQL runtime server during requirements gathering and have it return an empty string or dummy value.

The main issue with this solution is that runtime errors that occur due to code that contains side-effects will be linked to having an incorrect connection string rather than there being no database to reference at collection time.

Option 2 - Allow application level nitric lifecycle execution

This is already possible using env variables, but implementing this at the SDK level would make this more convenient.

A working example without SDK enhancement:

if (process.env.NITRIC_ENVIRONMENT == "build") {
   // Code to execute at build time
}

if (process.env.NITRIC_ENVIRONMENT == "run") {
  // Code the execute during local simulation
}

if (process.env.NITRIC_ENVIRONMENT == "cloud") {
  // Code the execute during deployed runs
}

In the specific example linked above using the getConnection() method as an example:

import 'package:nitric_sdk/nitric.dart';
import 'package:nitric_sdk/src/api/sql.dart';
import 'package:postgres/postgres.dart';

SqlDatabase get db => Nitric.sql(
      'notes',
      migrations: "file://db/migrations/notes",
    );

Future<Connection?> getConnection() async {
  // We need to ensure that db is reference regardless of the execution phase so it is evaluated
  final dbRef = db;
  final nitricEnv = Platform.environment['NITRIC_ENVIRONMENT'];

  if (nitricEnv == "build") {
    print('Skipping connection during nitric build');
    return null;
  }

  final uri = Uri.parse(await db.connectionString());

  final userInfoParts = uri.userInfo.split(':');
  final username = userInfoParts.length == 2 ? userInfoParts[0] : null;
  final password = userInfoParts.length == 2 ? userInfoParts[1] : null;
  final isUnixSocketParam = uri.queryParameters['is-unix-socket'];
  final applicationNameParam = uri.queryParameters['application_name'];
  final endpoint = Endpoint(
    host: uri.host,
    port: uri.port,
    database: uri.path.substring(1),
    username: username ?? uri.queryParameters['username'],
    password: password ?? uri.queryParameters['password'],
    isUnixSocket: isUnixSocketParam == '1',
  );

  final settings = ConnectionSettings(
    applicationName: applicationNameParam,
    sslMode: SslMode.values.byName(uri.queryParameters['sslmode'] ?? 'disable'),
  );

  return Connection.open(endpoint, settings: settings);
}

While more verbose that Option 1, the advantage is that the reason for this behavior is documented with the application code.

chimon2000 commented 6 days ago

Option 2 seems to be the better option since this can be abstracted into the current SDK implementation and it seems resilient to the side effects identified in option 1

tjholm commented 1 day ago

Just putting a rough proposal for an SDK level abstraction of the above code but using typescript as an example:

import * as nitric from "@nitric/sdk";

nitric.atRuntime(() => {
  console.log("runtime");
});

nitric.atBuildTime(() => {
  console.log("build");
});