lightninglabs / lightninglabs/aperture

meterd+proxy: close the metering test gaps from the #247 review

Open
#255 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement metering
Dominant language
Go
Stars
268
Forks
78
Avg merge
22h 25m
Merged PRs (30d)
1

Description

Two coverage gaps the review of #247 identified. Neither is a bug today; both are places where a silent regression would not be caught.

## The ErrorHandler wiring is untested

#247 added an `ErrorHandler` to the `ReverseProxy` that releases the reservation when the backend never produces a response. The release helper itself is tested (`TestReleaseReservationOnAbandonedRequest`), but nothing drives the actual proxy against a dead backend to prove the metering context value survives into the handler.

It does today: `ReverseProxy` builds `outreq := req.Clone(ctx)` from the incoming request, so the context carries through and the `meteringContextKey` lookup finds the info. That is a stdlib behavior we depend on and do not pin. A `httptest`-backed test that points a metered service at a closed port and asserts a usage report arrives would fix that.

## The new InvalidArgument paths are only tested one layer down

#247 made `GetPrice`, `ChallengeMinted` and `AuthorizeRequest` fail closed when the request model cannot be determined from the serialized body. That behavior is covered at the `modelFromRequestText` table level, but no `server_test.go` case asserts the RPC-level codes.

Worth pinning specifically because the guard's *placement* matters as much as its existence: the check in `AuthorizeRequest` sits before `s.store.Authorize`, which is what makes erroring safe (no reservation has been taken yet, so there is nothing to strand). A test asserting `codes.InvalidArgument` and an unchanged bundle balance would catch a future reordering.

## Acceptance

- A metered request to an unreachable backend produces a usage report carrying the echoed reservation.
- `GetPrice` / `ChallengeMinted` / `AuthorizeRequest` return `InvalidArgument` on an indeterminate model, and the bundle's reserved balance is unchanged afterwards.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the ReverseProxy and its existing TestReleaseReservationOnAbandonedRequest, then add an httptest-backed case for a metered request to an unreachable backend and verify the usage report carries the echoed reservation. In server_test.go, cover GetPrice, ChallengeMinted, and AuthorizeRequest with an indeterminate model; done means each returns codes.InvalidArgument and the bundle's reserved balance is unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend-api-design, testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.