JobListTx missing similar error to JobList on metadata w/ SQLite

[travisby]

Howdy!

I'm testing out the SQLite integration. We use metadata to provide an easy way to find particular jobs when we don't know the ID ahead of time.

Our code uses JobListTx rather than JobList, and we receive this fun error from the driver:

SQL logic error: unrecognized token: "@" (1)

I went to write a reproducer, and accidentally used JobList. To my surprise, there was a much better error!

JobListResult.Metadata is not supported on SQLite

Looking at the code, it looks like the error was added to JobList but not JobListTx.

Full reproducers below:

NOT GREAT ERROR:

package main

import (
	"context"
	"database/sql"
	"encoding/json"
	"log"

	"github.com/riverqueue/river"
	"github.com/riverqueue/river/riverdriver/riversqlite"
	"github.com/riverqueue/river/rivermigrate"
	_ "modernc.org/sqlite"
)

type job struct{}

func (j job) Kind() string { return "job" }

func main() {
	db, err := sql.Open("sqlite", ":memory:")
	if err != nil {
		log.Fatal(err)
	}
	defer db.Close()

	db.SetMaxOpenConns(1)

	driver := riversqlite.New(db)

	migrator, err := rivermigrate.New(driver, nil)
	if err != nil {
		log.Fatal(err)
	}

	ctx := context.Background()
	if _, err := migrator.Validate(ctx); err != nil {
		log.Fatal(err)
	}

	if _, err := migrator.Migrate(ctx, rivermigrate.DirectionUp, nil); err != nil {
		log.Fatal(err)
	}

	riverClient, err := river.NewClient(driver, &river.Config{})
	if err != nil {
		log.Fatal(err)
	}

	metadata, err := json.Marshal(map[string]string{
		"foo": "bar",
	})
	if err != nil {
		log.Fatal(err)
	}

	if _, err := riverClient.Insert(ctx, job{}, &river.InsertOpts{Metadata: metadata}); err != nil {
		log.Fatal(err)
	}

	tx, err := db.Begin()
	if err != nil {
		log.Fatal(err)
	}
	defer tx.Rollback()

	jobs, err := riverClient.JobListTx(ctx, tx, river.NewJobListParams().Metadata(string(metadata)))
	if err != nil {
		log.Fatal(err)
	}

	if err := tx.Commit(); err != nil {
		log.Fatal(err)
	}

	for _, j := range jobs.Jobs {
		log.Printf("job: %+v", j)
	}
}

VS the GREAT ERROR:

package main

import (
	"context"
	"database/sql"
	"encoding/json"
	"log"

	"github.com/riverqueue/river"
	"github.com/riverqueue/river/riverdriver/riversqlite"
	"github.com/riverqueue/river/rivermigrate"
	_ "modernc.org/sqlite"
)

type job struct{}

func (j job) Kind() string { return "job" }

func main() {
	db, err := sql.Open("sqlite", ":memory:")
	if err != nil {
		log.Fatal(err)
	}

	db.SetMaxOpenConns(1)

	driver := riversqlite.New(db)

	migrator, err := rivermigrate.New(driver, nil)
	if err != nil {
		log.Fatal(err)
	}

	ctx := context.Background()
	if _, err := migrator.Validate(ctx); err != nil {
		log.Fatal(err)
	}

	if _, err := migrator.Migrate(ctx, rivermigrate.DirectionUp, nil); err != nil {
		log.Fatal(err)
	}

	riverClient, err := river.NewClient(driver, &river.Config{})
	if err != nil {
		log.Fatal(err)
	}

	metadata, err := json.Marshal(map[string]string{
		"foo": "bar",
	})
	if err != nil {
		log.Fatal(err)
	}

	if _, err := riverClient.Insert(ctx, job{}, &river.InsertOpts{Metadata: metadata}); err != nil {
		log.Fatal(err)
	}

	jobs, err := riverClient.JobList(ctx, river.NewJobListParams().Metadata(string(metadata)))
	if err != nil {
		log.Fatal(err)
	}

	for _, j := range jobs.Jobs {
		log.Printf("job: %+v", j)
	}
}

[EDIT] depending on how quickly https://github.com/riverqueue/river/issues/570#issuecomment-2869139374 comes into play, this might be a non-issue!

[brandur]

Thanks for the report. Sending a fix in #924.

Out of curiosity, are you actually using JobListParams.Metadata?

[travisby]

Sending a fix in https://github.com/riverqueue/river/pull/924.

🙌 wooh! That was fast!

Out of curiosity, are you actually using JobListParams.Metadata?

Yep! That's the only JobListParam I think I actually use! (I didn't even filter by Kind(), accidentally 🙈 )

[travisby]

In particular,

I set a "CallbackID" in metadata for a job, because I need to send a webhook about the job that hasn't been created yet (and I don't know its ID, and honestly want it to be un-guessable by using a UUID). On a later request, we find the job by that callback ID (in the metadata).

e.g.

1. Generate callback ID
2. Create job (with metadata containing callback ID)
3. Call an external service, while giving them the callback ID.
4. wait a little while
5. receive callback from external service that contains the callback ID
6. Look up the job via callback ID (metadata search), do something with that job

I particularly chose Metadata because it was an available filter, compared to something in the JobArgs. I'm not married to Metadata if a future way to filter like that becomes available!

[brandur]

@travisby Gotcha, Okay thanks for explaining! I gotta say that is somewhat bad luck ... I think every feature reimplemented in SQLite (many required some pretty big workarounds but we got there), except this one relating to metadata where there was just no way I could find whatsoever to do it. I'll have to look into that JSON Path alternative sooner rather than later.

[brandur]

Alright, I took a shot at implementing an escape hatch in #933 so that at least there's a way of duplicating similar functionality under SQLite. I think more generally, we'll probably want to avoid implementing helpers like Metadata that are reliant on specific database features, and have users bring in their own implementations. I originally built a general helper that'd combine JSON Path with a basic equality operator, but found that there was still enough differences between databases that using it required a lot of driver-level change and still had footguns.