Skip to content

Fix UPS and USPS time parsing - #35

Closed
aldin-alagic wants to merge 2 commits into
egg-:masterfrom
uxdotapp:feature/change-ups-and-usps-time-parsing
Closed

Fix UPS and USPS time parsing#35
aldin-alagic wants to merge 2 commits into
egg-:masterfrom
uxdotapp:feature/change-ups-and-usps-time-parsing

Conversation

@aldin-alagic

Copy link
Copy Markdown

Bug fixes:

  • Fix the UPS tracker incorrectly parsing the 12-hour time component due to missing 'a' token
  • Fix the USPS tracker not parsing time correctly due to extra spaces in the middle of the time string

@dav72

dav72 commented Jan 2, 2023

Copy link
Copy Markdown

I have tried to implement this thing on my usps tracking site at Usps tracking , but almost all of trackers not working except usps , Canada post. Ups partially work. India post, fedex kind of not working.

@egg-

egg- commented Aug 5, 2026

Copy link
Copy Markdown
Owner

@aldin-alagic I owe you an apology — this sat here for three years and both bugs were real.

While rewriting the library in #42 I ran the old and new versions side by side over every recorded fixture to make sure nothing changed behaviour. The UPS bug slipped straight through that check, because both versions were wrong in exactly the same way. I only found it when I came back to this PR. So your report caught something my own testing couldn't.

For anyone reading later, the UPS one is worse than it sounds: UPS returns 10:53 P.M. and the code parsed it as HH:mm, so the meridiem was thrown away and every afternoon event was recorded twelve hours early. Seven of the twelve activities in our test fixture are P.M., so it was wrong more often than it was right.

I've put both fixes up as #43. I couldn't merge this PR directly — v3 moved the sources from lib/ to src/ and your patch no longer applies — so I reapplied the changes there and credited you in the commit and the changelog. One adjustment was needed: v3 uses dayjs instead of moment, and dayjs only matches PM, not P.M., so the periods have to be stripped before parsing. moment was lenient about that, which is why your version worked as written.

I also added a regression test for each. The suite had a check that timestamps parse, but a twelve-hour error passes that cleanly, so it needed asserting the actual hour.

Closing in favour of #43. Thanks for the fix, and sorry it took this long to act on it.

egg- added a commit that referenced this pull request Aug 5, 2026
UPS reports a 12-hour clock as "10:53 P.M.", but the format string read it
as `HH:mm`, so the meridiem was ignored and every afternoon event landed
twelve hours early. Seven of the twelve activities in the recorded fixture
are P.M., so this was wrong far more often than not.

dayjs only matches "PM", not "P.M.", so the periods have to be stripped
before parsing with `h:mm A`.

USPS gets the same treatment for whitespace: runs of spaces inside the date
cell are collapsed before parsing, and the format list now also accepts a
space after the second comma.

Both were reported in #35 by @aldin-alagic in 2022. That PR targeted the old
`lib/` tree and could no longer be merged after the v3 rewrite, so the fixes
are reapplied here with a regression test for each — the existing sweep only
checked that a timestamp parsed, not that it landed on the right hour.
@egg- egg- closed this in #43 Aug 5, 2026
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