Skip to content

[WIP] Fix crash in Nakama due to missing IP cache configuration#142

Draft
Copilot wants to merge 22 commits intomainfrom
copilot/fix-ip-cache-crash
Draft

[WIP] Fix crash in Nakama due to missing IP cache configuration#142
Copilot wants to merge 22 commits intomainfrom
copilot/fix-ip-cache-crash

Conversation

Copy link
Contributor

Copilot AI commented Nov 6, 2025

Fix Nakama crash when IP cache is not configured

  • Analyze the issue and understand the root cause
  • Modify evr_pipeline.go to handle missing IP cache providers gracefully
  • Ensure ipInfoCache is always initialized (even with no providers)
  • Add warning logs instead of fatal errors
  • Create tests to verify the fix
  • Verify the changes work correctly
  • Format code with gofmt
  • Address code review feedback

Summary

Fixed potential crash when Redis is not configured by:

  • Adding warning log when Redis client is not configured
  • Adding warning log when no IP providers are configured
  • Making it explicit that server can run without IP cache
  • Created comprehensive tests for IP cache with no providers
  • Fixed bug where IPAPI_API_KEY was checked instead of IPQS_API_KEY

The IP cache now gracefully handles missing providers:

  • Get() returns nil (no error) when no providers configured
  • IsVPN() returns false (safe default) when no providers configured
  • All code paths verified to handle nil ipInfo correctly
Original prompt

This section details on the original issue you should resolve

<issue_title>Nakama crashes if IP cache is not configured in server/evr_pipeline.go</issue_title>
<issue_description>### Summary
Nakama crashes when an IP cache is not configured. This occurs in server/evr_pipeline.go. The crash is due to the lack of handling for a nil or missing IPInfoProvider configuration, leading to a fatal error when initializing the IP info cache.

Steps to Reproduce

  1. Run Nakama without configuring an IP cache (no Redis client, no IPAPI/IPQS keys).
  2. Application crashes on startup.

Error Location

The problematic code is in server/evr_pipeline.go:

	var providers []IPInfoProvider
	if redisClient != nil {
		if vars["IPAPI_API_KEY"] != "" {
			ipqsClient, err := NewIPQSClient(logger, metrics, redisClient, vars["IPQS_API_KEY"])
			if err != nil {
				logger.Fatal("Failed to create IPQS client", zap.Error(err))
			}
			providers = append(providers, ipqsClient)
		}

		ipapiClient, err := NewIPAPIClient(logger, metrics, redisClient)
		if err != nil {
			logger.Fatal("Failed to create IPAPI client", zap.Error(err))
		}
		providers = append(providers, ipapiClient)
	}
	ipInfoCache, err = NewIPInfoCache(logger, metrics, providers...)
	if err != nil {
		logger.Fatal("Failed to create IP info cache", zap.Error(err))
	}

Proposed Fix

Add logic to handle the case where no providers are configured (e.g., redisClient is nil), so Nakama does not crash. This could involve initializing an empty provider slice or logging a warning instead of calling logger.Fatal.

Version

Please specify the Nakama version where this occurs.

Additional Context

  • File: server/evr_pipeline.go
  • The crash occurs at startup if no IP cache provider is set.

Labels: bug, crash
Issue Type: Bug</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

thesprockee and others added 20 commits October 15, 2025 12:40
- Implement BeforeWriteStorageObjectsHook and BeforeDeleteStorageObjectsHook to enforce StorageObjects intent (or IsGlobalOperator) and block unauthorized write/delete requests by returning nil.
- Clarify AfterReadStorageObjectsHook comment to specify retry when zero objects were returned.
- Small TODO placement fix in BeforeListMatchesHook.
- Comment out registration of the BeforeWriteStorageObjects guard in registerAPIGuards.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Bates <a@sprock.io>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
* Initial plan

* Add nakama-debug to .gitignore

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
…tion (#124)

* Initial plan

* Extract common storage authorization logic into helper function

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
…ing (#127)

* Initial plan

* Add novalidation tag support for game server registration

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Add documentation for novalidation tag

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Use constant for novalidation tag to avoid magic string

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Make audit message sending async and clarify documentation

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
…ions (#128)

* Initial plan

* Extract common authorization logic into helper function

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Signed-off-by: Andrew Bates <a@sprock.io>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
…130)

* Initial plan

* Add nakama-debug to .gitignore

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Conditionally show moderator Discord ID in suspension embeds

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Add tests for conditional enforcer ID display in suspension embeds

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
* Initial plan

* Initial analysis - Identify error message improvement needed

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Update error messages to include internal IP, external IP, and port

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
Use the loop variable (gID) instead of the outer groupID when calling
createSuspensionDetailsEmbedField so each embed field references the
correct guild instead of always using the current group.
* Initial plan

* Initial plan for fixing Past Display Names embed bug

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Fix Past Display Names embed bug and add missing ServerEventsPublish method

- Fixed timestamp assignment bug in createPastDisplayNameEmbed where variable 'e' was being assigned instead of 'ts'
- Added comprehensive test coverage for createPastDisplayNameEmbed function
- Fixed Satori build issue by implementing missing ServerEventsPublish method

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
* Added IGN Override lock toggle and interaction for user metadata

* Update server/evr_discord_appbot_handlers.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Bates <a@sprock.io>

* Update server/evr_discord_appbot_igp.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Bates <a@sprock.io>

* Update server/evr_discord_appbot.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Bates <a@sprock.io>

---------

Signed-off-by: Andrew Bates <a@sprock.io>
Co-authored-by: Andrew Bates <a@sprock.io>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Bates <a@sprock.io>
…gs (#131)

* Initial plan

* Add daily average player loudness leaderboard stat

- Add PlayerLoudnessStatisticID constant
- Process VOIP_LOUDNESS remote logs to accumulate loudness data
- Track total loudness, min, max, and count per player
- Store metadata (min_loudness, max_loudness, count) in leaderboard records
- Queue leaderboard updates with daily reset schedule
- Add unit tests for loudness accumulation and average calculation
- Fix satori interface compilation issue with stub method

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Add documentation for PlayerLoudness leaderboard stat

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Fix metadata type handling and documentation format

- Fix metadata parsing to handle string values correctly (was trying to parse as float64)
- Remove incorrect comment syntax from markdown documentation
- Add test verification for metadata parsing

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Add error handling for metadata parsing and fix documentation

- Add error handling for fmt.Sscanf calls with warning logs
- Update documentation example to show correct string-based metadata parsing
- Ensures parsing failures are logged instead of silently failing

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Refactor to use strconv instead of fmt.Sscanf for parsing

- Replace fmt.Sscanf with strconv.ParseFloat/ParseInt for better performance
- Update all code and documentation to use strconv functions
- More appropriate and efficient for string-to-number conversion
- All tests still pass with the new approach

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Improve error messages and clarify comments

- Add more context to error messages for debugging
- Include match ID in player not found error
- Include team info in non-playing team error
- Clarify that average calculation happens when reading data, not storing

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Fix error message format and improve function documentation

- Fix format string in team validation error (removed missing %d placeholders)
- Update ServerEventsPublish comment to follow Go documentation conventions

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

* Remove satori modifications - not needed for this feature

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Co-authored-by: Andrew Bates <a@sprock.io>
Copilot AI and others added 2 commits November 6, 2025 13:15
- Add warning log when Redis client is not configured
- Add warning log when no IP providers are configured
- Remove fatal error that crashes server on startup
- IP cache is still initialized with empty providers and works correctly

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
- Fix IPAPI_API_KEY -> IPQS_API_KEY bug in provider check
- Use nil for metrics in tests (not used by tested methods)
- Remove unused LocalMetrics struct initialization

Co-authored-by: thesprockee <962164+thesprockee@users.noreply.github.com>
Copilot AI requested a review from thesprockee November 6, 2025 13:44
Base automatically changed from evr.202 to main November 11, 2025 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants