Do not replace registry errors with "<port> does not exist" - #2095
Do not replace registry errors with "<port> does not exist"#2095Bugale Bugalit (bugale) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes misleading error reporting in the versioned registry port lookup path by preserving and surfacing the actual registry error returned from RegistryImplementation::get_port_entry(), instead of replacing it with a generic "<port> does not exist" message.
Changes:
- In
VersionedPortfileProviderImpl::entry(), propagate theExpectedLerror (std::move(entry).error()) into the cache rather than formattingmsgPortDoesNotExist. - As a result, registry-level failures (git/network/filesystem/locking) now reach users directly via the existing
ExpectedLerror propagation path.
| msg::format(msgPortDoesNotExist, msg::package_name = name)) | ||
| .first; | ||
| entry_it = m_entry_cache.emplace(name.to_string(), std::move(entry).error()).first; | ||
| } |
There was a problem hiding this comment.
Is this whole thing now just:
entry_it = m_entry_cache.emplace(name.to_string(), reg->get_port_entry(name)).first;I believe what this was intending to do was preserve the error cases and map the "nullptr unique_ptr" case to the "port does not exist" error message. So I think it's good to not drop the "child" error but I think this is incorrectly dropping a legitimate error too.
There was a problem hiding this comment.
I missed that you already discussed this in the description but the "is this whole thing now just" comment remains
There was a problem hiding this comment.
Billy O'Neal (@BillyONeal)
Yeah, you're right. Once the error is kept, both branches store the same thing, so the if/else does nothing. Pushed your version.
The nullptr case still gets the "does not exist" message, just one level up: entry() is only called from load_control_file, and that checks !ent->get() right after unwrapping. So a genuinely missing port reads the same as before, and a git or network failure now says what actually broke.
Oh I see my comment is addressed in the description
…rosoft#2094) `VersionedPortfileProviderImpl::entry` discarded the error returned by `get_port_entry` and substituted `msgPortDoesNotExist`, so registry failures (network, git, filesystem) were reported as the port not existing. The genuinely-missing-port case is a *value* holding a null pointer, and is already handled by `load_control_file`; this `else` branch is only reached when there is a real error. Cache the `ExpectedL` as-is so it survives.
fb7e7dc to
2ca3cf7
Compare
Fixes #2094.
VersionedPortfileProviderImpl::entry()threw away the error returned byget_port_entryand substituted{package_name} does not exist, so any registry-level failure was reported as the port not existing.get_port_entryreturnsExpectedL<std::unique_ptr<RegistryEntry>>with three outcomes:Case 2 is already handled downstream in
load_control_file:so the
elsebranch touched here is reached only for case 1. It also usedmsg::formatrather thanmsg::format_error, so the line wasn't even prefixed witherror:.Before / after
This hid #2066 from us entirely. A concurrent
git initrace in the registries cache madeget_port_entryfail, and the entire user-visible output was:No
error:prefix, no git output — just every port from the default registry declared non-existent, which points suspicion at the manifest or the baseline instead of at the registry fetch that actually failed. Finding the real message required aGIT_TRACE2_EVENTcapture of a live failure:With this change that message reaches the user directly.
Worth noting: #2066 hit the same underlying failure but reached it through
get_baseline_version, which propagates errors correctly — so that reporter saw the actual git error. The difference was entirely this oneelsebranch.The change
ExpectedT::error() &&already exists and the cache value type is constructible fromLocalizedString, so this is a drop-in replacement for the discardedmsg::format(...).msgPortDoesNotExistremains in use for the genuine cases at both other call sites in this file.