Skip to content

DatasetDao: map JDBC driver NPE on versionSchema column read to NotFound - #3258

Open
nicolaslopezbravo wants to merge 4 commits into
masterfrom
nbravo/dataset-dao-npe-to-notfound
Open

DatasetDao: map JDBC driver NPE on versionSchema column read to NotFound#3258
nicolaslopezbravo wants to merge 4 commits into
masterfrom
nbravo/dataset-dao-npe-to-notfound

Conversation

@nicolaslopezbravo

@nicolaslopezbravo nicolaslopezbravo commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

A dataset GET request returned HTTP 500 in prod (2026-04-29) because mysql-connector-java 8.0.21 threw NullPointerException from ResultSetImpl.findColumn -> getInt while reading the versionSchema column, and the exception bubbled past executeGetDatasetStatement uncaught.

  • Wrap the resultSet.getInt(VERSION_SCHEMA) call with a narrow NullPointerException catch and translate it to AccountServiceException(NotFound). AccountAndContainerInjector already maps AccountServiceErrorCode.NotFound -> RestServiceErrorCode.NotFound (HTTP 404).
  • Apply the same narrow catch to executeGetVersionSchema, which makes the identical JDBC call from the version-mutation paths (deleteDatasetVersion, deleteDatasetVersionForDatasetDelete, getAllValidVersionForDatasetDeletion, listAllValidDatasetVersions). Not observed in prod on those paths, but the call shape is identical -- guard symmetrically.
  • Add MySqlMetrics.datasetRowReadNpeCount so on-call can distinguish JDBC-driver NotFounds from legitimate ones on dashboards.
  • The catch is intentionally narrow (only getInt(VERSION_SCHEMA), not objectMapper.readValue or enum-ordinal indexing) so unrelated NPEs aren't masked as NotFound.
  • Comment in the catch points at gradle/dependency-versions.gradle and asks the next person bumping mysql-connector-java to revisit.

Durability Risk

Read-only / metadata-mutation paths; no blob data is at risk and no replication semantics change. Mapping a transient driver NPE to NotFound is semantically slightly off (the row exists) but the GET dataset caller (AccountAndContainerInjector.setTargetDatasetAndVersionInRestRequestIfNeeded) does not branch on the response to do destructive create-if-missing. On the version-mutation paths, a transient NPE surfacing as NotFound is the same outcome a genuinely missing version would produce.

Testing Done

  • DatasetDaoTest.testGetDatasetMapsJdbcNpeOnVersionSchemaToNotFound -- drives getDataset, asserts NotFound, identifier propagation in the exception message, and datasetRowReadNpeCount increment.
  • DatasetDaoTest.testDeleteDatasetVersionMapsJdbcNpeOnVersionSchemaToNotFound -- drives deleteDatasetVersion, asserts the same on the executeGetVersionSchema path.
  • ./gradlew :ambry-account:test --tests DatasetDaoTest -- 2/2 PASSED.

nbravo and others added 4 commits May 7, 2026 16:39
In prod, GET /named/<account>/<container>/<dataset>
returned HTTP 500 because mysql-connector-java 8.0.21 threw NPE from
ResultSetImpl.findColumn -> getInt while reading the dataset row, and
the exception bubbled past executeGetDatasetStatement uncaught. Wrap
the column-read block with a narrow NPE catch and translate it to
AccountServiceException(NotFound), which AccountAndContainerInjector
already maps to RestServiceErrorCode.NotFound (HTTP 404).

Add DatasetDaoTest covering the NPE -> NotFound mapping.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Narrow the NullPointerException catch in DatasetDao.executeGetDatasetStatement
  to only the resultSet.getInt(VERSION_SCHEMA) call (the call observed in the
  prod stack trace). The previous wider catch could also swallow NPEs from
  Jackson's objectMapper.readValue and from enum-ordinal array indexing,
  masking unrelated bugs as NotFound.
- Add a class logger to DatasetDao and emit a warn log with account/container/
  dataset identifiers when the NPE fires, so on-call has a grep target.
- Add MySqlMetrics.datasetRowReadNpeCount, incremented in the catch, so the
  failure mode is visible on dashboards even after it surfaces as a 404.
- Test: drop unused @RunWith(MockitoJUnitRunner.class), assert the exception
  message carries the dataset identifiers, and verify the new counter
  increments by 1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…he dataset

The frontend's setTargetDatasetAndVersionInRestRequestIfNeeded already emits
an error log with accountName/containerName/datasetName whenever the
AccountServiceException propagates up, so the warn log inside the catch
just duplicates the same identifiers a layer down. The new
datasetRowReadNpeCount counter is the dedicated signal for the JDBC NPE
path; the existing frontend log identifies which dataset.

Drop the warn log, the SLF4J Logger field, and the slf4j imports.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fix in executeGetDatasetStatement guards only the GET dataset path,
but executeGetVersionSchema makes the same resultSet.getInt(VERSION_SCHEMA)
call from the version mutation paths (deleteDatasetVersion,
deleteDatasetVersionForDatasetDelete, getAllValidVersionForDatasetDeletion,
listAllValidDatasetVersions). The mysql-connector-java 8.0.21 NPE has not
been observed there in prod, but the call shape is identical so guard it
symmetrically. Reuse datasetRowReadNpeCount for the metric.

Add DatasetDaoTest case driving deleteDatasetVersion to verify the same
NotFound mapping and counter increment.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@nicolaslopezbravo
nicolaslopezbravo force-pushed the nbravo/dataset-dao-npe-to-notfound branch from 59cbcf9 to f5f3450 Compare May 8, 2026 00:42
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.

1 participant