Skip to content

fix(consul): tolerate null Service.Meta in fetch_services_from_server#13258

Merged
Baoyuantop merged 3 commits intoapache:masterfrom
nic-6443:nic/fix-consul-meta-null
Apr 21, 2026
Merged

fix(consul): tolerate null Service.Meta in fetch_services_from_server#13258
Baoyuantop merged 3 commits intoapache:masterfrom
nic-6443:nic/fix-consul-meta-null

Conversation

@nic-6443
Copy link
Copy Markdown
Member

What this does

Fix a crash in the Consul service discovery worker when a registered service has no metadata.

Why

When consul services register is used without meta, the agent's catalog API returns "Meta": null. cjson decodes JSON null as a userdata sentinel (cjson.null), which is truthy but not a table. The current guard at apisix/discovery/consul/client.lua:415:

if preserve_metadata and node.Service.Meta
        and next(node.Service.Meta) then

passes the truthiness check, then calls next() on userdata and aborts the entire catalog scan with:

bad argument #1 to 'next' (table expected, got userdata)

The discovery worker never recovers — no nodes are produced for any service in that response, so all upstreams resolved through Consul end up empty until the worker is restarted (and even then it crashes again on the next scrape).

This was introduced by #13230 which extracted client.lua and added the new preserve_metadata path; older code didn't iterate Service.Meta.

Fix

Use type(node.Service.Meta) == "table" instead of relying on truthiness, so cjson.null is treated as "no metadata" (same as the field being absent).

Test

Added a regression test (t/discovery/consul.t TEST 14) that:

  1. registers a real consul service without Meta
  2. calls consul_client.fetch_services_from_server directly with preserve_metadata=true
  3. asserts no error and metadata == nil

The test fails on master with the userdata error and passes with the fix.

Copilot AI review requested due to automatic review settings April 20, 2026 08:28
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Apr 20, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a crash in the Consul discovery client when Consul returns "Meta": null (decoded by cjson as cjson.null userdata) by guarding metadata iteration with a type(...) == "table" check, and adds a regression test for this scenario.

Changes:

  • Guard next(node.Service.Meta) behind type(node.Service.Meta) == "table" when preserve_metadata=true.
  • Add a new test that registers a real Consul service without Meta and verifies fetch_services_from_server does not error and returns metadata == nil.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
apisix/discovery/consul/client.lua Prevents next() from being called on cjson.null userdata when Service.Meta is null.
t/discovery/consul.t Adds a regression test covering Consul services registered without Meta.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread t/discovery/consul.t
Comment thread t/discovery/consul.t
Comment thread t/discovery/consul.t Outdated
When a consul service is registered without metadata, the agent returns
"Meta": null in the catalog response. cjson decodes that as a userdata
sentinel (cjson.null) which is truthy but not a table, so the existing
guard `node.Service.Meta and next(node.Service.Meta)` calls next() on
userdata and aborts the whole catalog scan with:

  bad argument #1 to 'next' (table expected, got userdata)

The discovery worker never recovers and no upstream nodes are produced.

Use `type(node.Service.Meta) == "table"` so cjson.null is treated as
absent metadata. Adds a regression test that registers a metadata-less
service and calls fetch_services_from_server directly with
preserve_metadata=true.
@nic-6443 nic-6443 force-pushed the nic/fix-consul-meta-null branch from 7ef4c3a to 6a5144c Compare April 20, 2026 08:56
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Apr 20, 2026
@Baoyuantop Baoyuantop merged commit 4c67a31 into apache:master Apr 21, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants