Skip to content

Fix UnboundLocalError when OSM header has non-numeric sequence number - #315

Open
xovishnukosuri wants to merge 1 commit into
osmcode:masterfrom
xovishnukosuri:fix/replication-utils-unbound-seq-in-warning
Open

xovishnukosuri wants to merge 1 commit into
osmcode:masterfrom
xovishnukosuri:fix/replication-utils-unbound-seq-in-warning

Conversation

@xovishnukosuri

Copy link
Copy Markdown

Summary

  • get_replication_header() in src/osmium/replication/utils.py crashes with UnboundLocalError when an OSM file header contains a non-numeric osmosis_replication_sequence_number value.
  • The except ValueError handler logged seq instead of seqstr. Since seq is only assigned inside the try block and the assignment fails for non-numeric strings, seq is unbound at the point the warning is emitted.
  • Fix: replace seq with seqstr so the raw invalid string is reported, matching the intent of the warning message.
  • Two new tests cover the invalid-sequence and negative-sequence warning paths, both of which were previously untested.

Root cause

# before (line 60 in utils.py)
except ValueError:
    LOG.warning("Sequence id '%s' in OSM file header is not a number. Ignored.", seq)
    #                                                                              ^^^
    # seq is unbound here — UnboundLocalError at runtime
# after
except ValueError:
    LOG.warning("Sequence id '%s' in OSM file header is not a number. Ignored.", seqstr)
    #                                                                              ^^^^^^
    # seqstr holds the raw value that failed int() conversion

Test plan

  • test_get_replication_header_invalid_sequence — verifies the function returns sequence=None and logs the bad string without raising
  • test_get_replication_header_negative_sequence — verifies the existing negative-value path also works and logs correctly
  • All 359 existing tests continue to pass

🤖 Generated with Claude Code

…numeric

In get_replication_header(), the warning log for an invalid sequence number
referenced `seq` instead of `seqstr`. Because `seq` is only assigned inside
the try block and the assignment fails when the string is non-numeric, `seq`
is unbound at the point of the except handler, causing an UnboundLocalError
that completely silences the intended warning and crashes the function.

Replace `seq` with `seqstr` in the warning call so the raw string value that
failed parsing is reported. Add two tests that cover the invalid-sequence and
negative-sequence paths, both of which were previously untested.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant