Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

discovery: output.Cached() TTL check is inverted, cache never hits when valid

Open Beginner friendly
  • Is something wrong?
#124 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
Under an hour
Newbie friendliness
86/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go
Domain
backend

Research direction

Start with discovery/output.go:28-41 and check how Cached() compares the TTL with the current time; then review the callers in discovery/cache.go:58-60 and discovery/cache.go:77-79, especially the ReadyAt() behavior raised in the issue. Run the relevant discovery tests, or add focused coverage for valid and expired TTLs; done means valid cached devices are returned and expired cache data is not.

Written by the indexing model from the issue text.

Description

Summary

output.Cached() only returns the cached devices when the TTL has already expired, and returns nil when the TTL is still valid. So in normal operation the discovery cache never hits, and once it does expire it serves stale data as if it were fresh.

Ran into this while looking at the discovery output path after #123. The struct comment says it all, the check just does the opposite.

Location

discovery/output.go:28-41 (master @ 9f023d1)
Callers in discovery/cache.go:58-60 and discovery/cache.go:77-79

I searched for Cached, ttl, After and cache expiry — no overlap with #119, #121, #122, #123, #30 or #10.

Current behavior (with code)

The struct declares:

type output struct {
    ctx     context.Context // Logging context
    devices []*Device       // Cached output data
    ttl     time.Time       // Cache valid until this time
}

But Cached() checks it backwards:

func (out *output) Cached() []*Device {
    if out.devices != nil && !out.ttl.After(time.Now()) {
        return out.devices
    }
    return nil
}

ttl.After(now) means the TTL is still in the future, which means the cache is valid. The ! flips it, so a valid cache gives nil and an expired cache gives devices.

cache.Export() always sets a future TTL:

ttl := time.Now().Add(365 * 24 * time.Hour) // Far in a future
...
return c.out.Generate(ttl, units)

So in steady state Cached() deterministically returns nil. I ran the exact logic to be sure before filing: valid TTL with 365 days gives nil instead of devices, expired TTL gives devices instead of nil.

That breaks both callers. Export() never takes its fast path and regenerates everything on every GetDevices() call, with IP extraction plus variant merging plus DeviceID and UUID grouping plus sorting. ReadyAt() has the same inverted check for ModeWaitIncomplete, so it skips the staging wait only when the cache is expired.

Honestly the same file uses !After correctly for expiry in stagingCheck(), so this looks like that pattern got copied into a validity check where it does not belong.

Expected behavior

Cached() should return devices when ttl.After(now) is true, and return nil when the TTL has passed so the caller regenerates. That is just removing the !.

Possible fix

One character in output.go:

if out.devices != nil && out.ttl.After(time.Now()) {

I am leaving Invalidate() and Generate() alone, they already do the right thing.

Does that match your intent for ReadyAt() as well, or should the wait logic stay as is..?

Dominant language
Go
Stars
23
Forks
13
Avg merge
3h 35m
Merged PRs (30d)
10

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

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.

More from OpenPrinting/go-mfp

All issues in OpenPrinting/go-mfp

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.