Skip to content

FIX: Return the status of a failed reload as an error - #6

Closed
drasmart wants to merge 2 commits into
fix/device-detection/free-exception-messagesfrom
fix/device-detection/reload-status-errors
Closed

drasmart wants to merge 2 commits into
fix/device-detection/free-exception-messagesfrom
fix/device-detection/reload-status-errors

Conversation

@drasmart

@drasmart drasmart commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Carried over from device-detection-go#104 by James Rosewell, a draft with no review yet, while device-detection-go is being merged into this repository. The branch is brought in as is, with its original commits, so the changes are under device-detection/.


Stacked on #5. Merge that first, then retarget this to main.

Closes #4

Problem

ReloadFromFile and ReloadFromOriginalFile discarded the status code that the C reload returns and only checked the exception. A reload that failed with a status code, such as a missing file with the InMemory profile or a path that is too long, returned nil although the loaded data set was not replaced. #4 has the details and links.

Change

  • dd/resource_hash.go: both functions now check the status code and build the error with StatusGetMessage and messageError, the same way InitManagerFromFile does. ReloadFromFile names the file it was given. ReloadFromOriginalFile names the file the loaded data set came from, which is the file the C reload reads.
  • dd/reload_test.go adds TestReloadFromFileReportsStatus, which reloads a missing file into a manager loaded with the InMemory profile, and TestReloadFromOriginalFileReportsStatus, which loads a copy of the data file, deletes the copy and reloads from the original file. Both expect an error naming the file. They use the Lite data file that TestMain already requires, so they run wherever the package tests run.

Callers that ignored reload errors will now see them. A failed reload now returns, for example, The data file '<path>' could not be found. Check the file path and that the program has sufficient read permissions.

In the on-premise engine, reloadManager already returns these errors. A failed reload now stops before it deletes the previous temporary copy and before it records the new file as the one in use. A failure from a file watcher reload is logged. A failure from an automatic update reload, which happens only when the file watcher is off, ends the engine's reloadFileEvent loop, so the next download waits forever to hand over its reload. That loop already ends that way for other reload errors. This last point comes from reading onpremise/onpremise.go and common-go's file_pulling.go, not from a run.

Verification

Windows 11, Go 1.25.1, MSYS2 UCRT64 gcc 16.2.0, CGO_ENABLED=1, with 51Degrees-LiteV4.1.hash at the repository root and in assets/. "Before" is 4b5857d, the head of #5, and "after" is 1c51931.

Check Before After
go build -p 2 ./... passes passes
go vet -p 2 ./dd ./onpremise clean clean
go vet -p 2 ./dd ./onpremise with go.mod temporarily set to go 1.24 clean clean
go test -p 2 -count=1 ./dd passes passes
go test -p 2 -count=1 ./onpremise passes passes
go test -p 2 -count=1 -v -run TestReloadFrom ./dd TestReloadFromFileReportsStatus and TestReloadFromOriginalFileReportsStatus fail with "reported success" pass

For the "before" run, the new test file was run against 4b5857d. Before #5, a direct call of C.HashReloadManagerFromFile returned status 19 for a 65,617 character path and status 4 for a missing file, without an exception, and ReloadFromFile returned nil for both.

CI

The shared build and test workflow only picks up pull requests that are not drafts and that target main, so the build and test matrix has not run on this pull request. The UTM link lint is the only check that runs here.

What remains

  • The engine's reloadFileEvent loop ends at the first reload error. That is not changed here.
  • A local check, not committed, showed the loaded data set still in use after each failed reload. After both failures the manager matched a mobile User-Agent and returned True for IsMobile. The committed tests check only the errors.

jwrosewell and others added 2 commits October 26, 2026 23:10
ReloadFromFile and ReloadFromOriginalFile discarded the status code that
the C reload returns and only checked the exception. A reload that
failed with a status, such as a missing file with the in memory profile
or a path that is too long, returned nil although the loaded data set
was not replaced.

Both now build the error from StatusGetMessage the same way
InitManagerFromFile does. ReloadFromFile names the file it was given.
ReloadFromOriginalFile names the file the loaded data set came from,
which is the file the C reload reads.

Callers that ignored reload errors will now see them.

TestReloadFromFileReportsStatus and
TestReloadFromOriginalFileReportsStatus check both with the Lite data
file the package tests already use.
@drasmart

drasmart commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #7, now merged, which checks the status of both reloads in a shared reloadError.

@drasmart drasmart closed this Nov 2, 2026
@drasmart
drasmart deleted the fix/device-detection/reload-status-errors branch November 2, 2026 16:51
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.

2 participants